Skip to content

Implement exercise hangman#1690

Merged
cmccandless merged 9 commits intoexercism:masterfrom
junming403:exercise-hangman
Mar 4, 2019
Merged

Implement exercise hangman#1690
cmccandless merged 9 commits intoexercism:masterfrom
junming403:exercise-hangman

Conversation

@junming403
Copy link
Copy Markdown
Contributor

Fixes #745

Solution based on the work of #1160

Config file lint with tool configlet, shall I keep all the changes in the config?

Ready for review

[Hangman][] is a simple word guessing game.

[Hangman]: https://en.wikipedia.org/wiki/Hangman_%28game%29

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Since this file is supposed to be generated with configlet generate, I'm not comfortable with manually editing it, even to remove inaccurate (to this track) data.

Here are the options I can think of:

  • Add a note to this exercise's .meta/HINTS.md that tells users to ignore the part about a 3rd-party framework.
  • Leave inaccurate data as-is (least preferable)
  • Submit a PR to exercism/problem-specifications to remove the reference to a 3rd-party framework, leaving it to individual tracks to add that information (i.e. C# track does this already)
  • Mark this exercise as foregone since there is no builtin FRP framework. There is a precedent for this with the lens-person exercise.
  • [Insert other options here]

Copy link
Copy Markdown
Contributor Author

@junming403 junming403 Feb 27, 2019

Choose a reason for hiding this comment

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

Thank you for the feedback, I have added a hint about that.

Comment thread config.json
Comment thread exercises/hangman/hangman_test.py Outdated
Comment thread exercises/hangman/hangman_test.py Outdated
@junming403
Copy link
Copy Markdown
Contributor Author

I have updated according to the review, except the part regarding config file. Thank you!

Comment thread exercises/hangman/hangman.py Outdated
Comment thread exercises/hangman/hangman_test.py
@junming403
Copy link
Copy Markdown
Contributor Author

Sorry for the careless mistake, I have fixed them, ready for review. Thank you.

@junming403
Copy link
Copy Markdown
Contributor Author

Hi Sir, Would you mind help me review this again? @cmccandless

@cmccandless
Copy link
Copy Markdown
Contributor

Apologies for the silence.

I've been struggling with whether to include this exercise at all, since we are intentionally ignoring the exercise purpose as stated in its README (FRP). I think for now, we will go ahead and merge it in, with the option to redesign it later if we find a good solution for simulating FRP.

Copy link
Copy Markdown
Contributor

@cmccandless cmccandless left a comment

Choose a reason for hiding this comment

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

Just a couple other minor things that I noticed in my last review. We'll be good to go after these.

Comment thread config.json Outdated
Comment thread config.json Outdated
@junming403
Copy link
Copy Markdown
Contributor Author

Thank you for your review, I have updated the config files.
Is there any other exercises I can help with?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants