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

Implement black-jack concept exercise #2592

Merged
merged 7 commits into from
Sep 29, 2021

Conversation

pranasziaukas
Copy link
Contributor

@pranasziaukas pranasziaukas commented Sep 21, 2021

Partially addresses #2428

Implemented

  • instructions
  • code skeleton
  • tests
  • example solution
  • hints

as seemed appropriate.

Relied on https://bicyclecards.com/how-to-play/blackjack for some typical house rules.

Overall, seems in-line with teaching python operators.

Copy link
Contributor Author

@pranasziaukas pranasziaukas left a comment

Choose a reason for hiding this comment

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

Refactored the code to not use lists.

Copy link
Member

@BethanyG BethanyG left a comment

Choose a reason for hiding this comment

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

I left some suggestions and comments. Since this is a draft, I am going to leave them optional and open for discussion. Thank you for submitting this! 🌟

@BethanyG
Copy link
Member

@pranasziaukas -- in relation to this, would you also like to pick up PR2456, which is the concept write-up for this exercise?
It is perfectly fine if you do not want to, but we also can't mark this concept exercise live without it, so I am noting that fact here.

@BethanyG BethanyG linked an issue Sep 23, 2021 that may be closed by this pull request
3 tasks
@pranasziaukas
Copy link
Contributor Author

Thank you for reviewing @BethanyG. Since I'm not a native English speaker I gladly accepted the suggested phrasing changes.

in relation to this, would you also like to pick up PR2456, which is the concept write-up for this exercise?

I might pick that one up a bit later. In any case

  1. Should the concept write-up work be part of this or a separate PR?
  2. Should this remain as a draft PR?

@BethanyG
Copy link
Member

Should the concept write-up work be part of this or a separate PR?

It doesn't have to be. It depends on how you want to work. If it feels better to do it in a follow-on PR, that's completely fine. If you do decide to include that work in this PR, I would then make this a large-size PR, and keep it in draft form until we get those concept write-up pieces done.

Should this remain as a draft PR?

Only if you want to include the concept write-up. If not, we can change this to full PR and get it reviewed and merged. The exercise won't go "live" on the site however until the concept introduction.md and about.md write-ups are complete, so we don't have TODO showing on tooltips and pages.

Just let me know how you want to proceed. 😄

@pranasziaukas
Copy link
Contributor Author

Great!

I guess I prefer working in smaller chunks. Therefore, I'm marking this PR as ready for review as it is.
Once the code is merged, it's only the concept write-ups that need to be done, and those can be naturally addressed in a separate PR.

@pranasziaukas pranasziaukas marked this pull request as ready for review September 28, 2021 15:38
Copy link
Member

@BethanyG BethanyG left a comment

Choose a reason for hiding this comment

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

@pranasziaukas - Thank you for this work! 🌟 I think that this PR picked up your follow-on test changes. So merging that one might get weird. Here is me crossing my fingers that it doesn't.

@BethanyG
Copy link
Member

aaand we have conflicts. SAD FACE. @pranasziaukas -- can you try fetching from head and rebasing this locally? If all else fails, we'll have to force it, but I am hoping you can resolve this without that. Thanks! 😄

@pranasziaukas pranasziaukas force-pushed the black-jack-implementation branch from 3b1ee78 to 1daf2a9 Compare September 29, 2021 04:38
@pranasziaukas
Copy link
Contributor Author

Should be good now.

The conflict was class BlackJackTest(unittest.TestCase) vs class TestBlackJack(unittest.TestCase) -- I don't quite know/remember where that came from :)

@BethanyG
Copy link
Member

It was me. 😞 I changed the name to BlackJackTest to make the output in the UI prettier. It was easier to change the place where Test was in the name than to make a regex that would reliably detect and replace it wherever it went. Now the results will print out with Blackjack > test _____ Apologies for that.

Thank you again for resolving things and for working on this!

@BethanyG BethanyG merged commit b795319 into exercism:main Sep 29, 2021
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.

[Improve Concept Exercise] : Black Jack (comparisons)
3 participants