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

[V3] Add Concept Exercise: While and Switch #1073

Merged
merged 24 commits into from
Apr 11, 2021

Conversation

junedev
Copy link
Member

@junedev junedev commented Apr 7, 2021

Addresses #991

Notes

  • Although there was no final decision yet (https://exercism-team.slack.com/archives/CR91YFNG3/p1616872980114600), I removed the alcohol from the original exercise to be on the save side.
  • The introduction includes break and continue because I find them essential to the concept. But currently you could solve the exercise without them. I didn't want to delay getting this ready by making up another task for this. I left a note in the design.md file that this would be a good addition.
  • I had use a prettier ignore directive for the switch statement in the docs because it was indenting the comment below the default statement the wrong way.
  • CI is failing because of a configlet update but the errors are not related to the new exercise. Resolved

@junedev junedev changed the title Add concept exercise while [V3] Add Concept Exercise: While and Switch Apr 7, 2021
@junedev junedev marked this pull request as ready for review April 8, 2021 18:04
@junedev junedev requested a review from a team April 8, 2021 18:04
@SleeplessByte SleeplessByte self-assigned this Apr 8, 2021
Copy link
Member

@tejasbubane tejasbubane left a comment

Choose a reason for hiding this comment

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

Minor suggestions.

concepts/while-loops/about.md Outdated Show resolved Hide resolved
concepts/while-loops/about.md Outdated Show resolved Hide resolved
concepts/while-loops/about.md Outdated Show resolved Hide resolved
exercises/concept/mixed-juices/.docs/instructions.md Outdated Show resolved Hide resolved
exercises/concept/mixed-juices/.docs/instructions.md Outdated Show resolved Hide resolved
Copy link
Member

@SleeplessByte SleeplessByte left a comment

Choose a reason for hiding this comment

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

This is looking great. As with all the while/loop/switch exercises in other languages, this one suffers that if you know recursion / if you know difficult / complex map reduces, it will be easier to solve that way, but I really like how you've made it likely enough that someone will solve this the intended way.

Analyzers will do their job here. We can have analyzers (as per your design document) shout if a while etc. is not used, so I think we can cover our cases there!

concepts/conditionals-switch/about.md Show resolved Hide resolved
concepts/conditionals-switch/about.md Outdated Show resolved Hide resolved
concepts/conditionals-switch/about.md Outdated Show resolved Hide resolved
concepts/conditionals-switch/introduction.md Show resolved Hide resolved
concepts/conditionals-switch/introduction.md Outdated Show resolved Hide resolved
exercises/concept/mixed-juices/.docs/instructions.md Outdated Show resolved Hide resolved
exercises/concept/mixed-juices/.docs/instructions.md Outdated Show resolved Hide resolved
exercises/concept/mixed-juices/.docs/introduction.md Outdated Show resolved Hide resolved
exercises/concept/mixed-juices/.meta/config.json Outdated Show resolved Hide resolved
@junedev
Copy link
Member Author

junedev commented Apr 10, 2021

Thanks for the reviews! One open question: #1073 (comment), all the other things are fixed. I also fixed all the code comments that were real sentences to be written as such (first letter capitalized, period at the end).

@SleeplessByte
Copy link
Member

Sweeet. Very nice. I've also answered the final open question.

@junedev
Copy link
Member Author

junedev commented Apr 11, 2021

From my point of view this is now ready to be merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants