-
-
Notifications
You must be signed in to change notification settings - Fork 657
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Need for Speed: Minor improvements (#1755) #2235
Conversation
Dear eklatzerThank you for contributing to the Go track on Exercism! 💙
Dear Reviewer/Maintainer
Automated comment created by PR Commenter 🤖. |
Hey, I adjusted the format of the outputs because I have seen #2202 |
The other changes should fix the TODO's from #1755 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm just going to suggest a change in the order we talk about these concepts. I think we should talk about string literals first and then the New()
or New***()
functions to create structs. I would leave the explanation of the new
builtin for last as it is not as relevant for the exercise .
Also note that the introduction.md
for the exercise is often a copy of the introduction.md
and about.md
of the concept they teach, in this case structs, so the changes must also be reflected there. You can leave this for last, just mentioning so it's not forgotten.
I hope I have covered everything you wanted me to change |
@andrerfcsantos I am a bit confused, have you already checked my changes, is something missing or what is the current state? |
I did take a look at this and I have some changes in mind, but I want to think about them a little bit better before actually suggesting them. This is why this is taking longer to review, didn't have the chance yet to fully gather and formalize my thoughts - but it is not forgotten! :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left my review in about.md
just to make things simpler, but please also apply the changes in the other files.
@andrerfcsantos Thanks for the feedback, I hope I have addressed all of your messages. Are more details needed in the |
Fixes #1755
Hey, I have done the following:
instructions.md
to clarify the struct-initialization and the usage of constructors