Skip to content
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

Concept stringers with exercise #2208

Merged
merged 13 commits into from
May 29, 2022
Merged

Conversation

norbs57
Copy link
Contributor

@norbs57 norbs57 commented Apr 29, 2022

Fixes #2184
New concept stringers
New concept exercise Meteorology

@github-actions
Copy link
Contributor

Dear norbs57

Thank you for contributing to the Go track on Exercism! 💙
You will see some automated feedback below 🤖. It would be great if you can make sure your PR covers those points. This will save your reviewer some time and your change can be merged quicker.

  • 📜 The following files usually contain very similar content.

    • concepts/<concept>/about.md
    • concepts/<concept>/introduction.md
    • exercises/concept/<exercise>/.docs/introduction.md

    Please check whether the changes you made to one of these also need to be applied to the others.

  • 🧦 If you changed the function signature or the function comment in the exemplar file or the stub file (<exercise>.go), make sure the change is applied to both files.

  • ✍️ If your PR is not related to an existing issue (and is not self-explaining like a typo fix), please make sure the description explains why the change you made is necessary.

  • 🔤 If your PR fixes an easy to identify typo, if would be great if you could check for that typo in the whole repo. For example, if you found Unicdoe, use "replace all" in your editor (or command line magic) to fix it consistently.

Dear Reviewer/Maintainer

  • 📏 Make sure you set the appropriate x:size label for the PR. (This also works after merging, in case you forgot about it.)

  • 🔍 Don't be too nit-picky. If the PR is a clear improvement compared to the status quo, it should be approved as clear signal this is good to be merged even if the minor comments you might have are not addressed by the contributor. Further improvement ideas can be captured in issues (if important enough) and implemented via additional PRs.

  • 🤔 After reviewing the diff in the "Files changed" section, take a moment to think about whether there are changes missing from the diff. Does something need to be adjusted in other places so the code or content stays consistent?

Automated comment created by PR Commenter 🤖.

Fixes exercism#2184
New concept stringers
New concept exercise Meteorology
@junedev junedev added the x:rep/large Large amount of reputation label Apr 29, 2022
Copy link
Contributor

@nahuakang nahuakang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First attempt at reviewing 👍 😃 Thanks for creating this! I'll read the test cases a bit later and have currently 1 question and 1 nitpick.

concepts/stringers/about.md Outdated Show resolved Hide resolved
exercises/concept/meteorology/.meta/design.md Outdated Show resolved Hide resolved
@norbs57
Copy link
Contributor Author

norbs57 commented Apr 30, 2022

Thanks Nahua!

Copy link
Contributor

@nahuakang nahuakang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests are 👍 I just have 1 more comment but maybe @junedev or @andrerfcsantos are more suitable to judge whether this comment from me is a good idea or not. Thanks again!

exercises/concept/meteorology/meteorology.go Show resolved Hide resolved
@andrerfcsantos andrerfcsantos added the status/awaiting-maintainer This pull request is waiting on one or more maintainers. label May 25, 2022
Copy link
Member

@andrerfcsantos andrerfcsantos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for making this exercise. What you have here is an excellent foundation for the exercise already, so well done!

I left some suggestions in the about.md file, but of course please make those changes in the other places were they are needed too.

And feel free to disagree with the suggestions :)

concepts/stringers/about.md Outdated Show resolved Hide resolved
concepts/stringers/about.md Outdated Show resolved Hide resolved
concepts/stringers/about.md Outdated Show resolved Hide resolved
concepts/stringers/about.md Outdated Show resolved Hide resolved
concepts/stringers/about.md Outdated Show resolved Hide resolved
exercises/concept/meteorology/.docs/instructions.md Outdated Show resolved Hide resolved
exercises/concept/meteorology/meteorology.go Show resolved Hide resolved
exercises/concept/meteorology/.meta/design.md Outdated Show resolved Hide resolved
exercises/concept/meteorology/.meta/design.md Outdated Show resolved Hide resolved
exercises/concept/meteorology/.meta/design.md Outdated Show resolved Hide resolved
@andrerfcsantos andrerfcsantos added status/awaiting-contributor This pull request is waiting on the contributor. and removed status/awaiting-maintainer This pull request is waiting on one or more maintainers. labels May 26, 2022
Update concepts/stringers/about.md

Co-Authored-By: André Santos <[email protected]>
Copy link
Member

@andrerfcsantos andrerfcsantos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some more changes

exercises/concept/meteorology/.docs/hints.md Outdated Show resolved Hide resolved
exercises/concept/meteorology/.vscode/settings.json Outdated Show resolved Hide resolved
exercises/concept/meteorology/.meta/design.md Outdated Show resolved Hide resolved
exercises/concept/meteorology/.docs/instructions.md Outdated Show resolved Hide resolved
exercises/concept/meteorology/meteorology_test.go Outdated Show resolved Hide resolved
exercises/concept/meteorology/meteorology_test.go Outdated Show resolved Hide resolved
exercises/concept/meteorology/meteorology_test.go Outdated Show resolved Hide resolved
concepts/stringers/about.md Outdated Show resolved Hide resolved
@norbs57
Copy link
Contributor Author

norbs57 commented May 29, 2022

I would like to pass on this PR - it would be great if someone else could finish it.

@andrerfcsantos
Copy link
Member

@norbs57 I can finish it up for you - no worries :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/awaiting-contributor This pull request is waiting on the contributor. x:rep/large Large amount of reputation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement New Concept Exercise: Stringers
4 participants