Skip to content

Add State of Tic-Tac-Toe Exercise - #651

Merged
mk-mxp merged 2 commits into
exercism:mainfrom
tomasnorre:add-state-of-tic-tac-toe-exercise
May 8, 2024
Merged

Add State of Tic-Tac-Toe Exercise#651
mk-mxp merged 2 commits into
exercism:mainfrom
tomasnorre:add-state-of-tic-tac-toe-exercise

Conversation

@tomasnorre

@tomasnorretomasnorre commented Feb 29, 2024

Copy link
Copy Markdown
Contributor

Todo:

  • Add Enum when we have PHP 8.1 as min requirement

@mk-mxpmk-mxp added x:action/create Work on something from scratch x:knowledge/elementary Little Exercism knowledge required x:module/practice-exercise Work on Practice Exercises x:size/large Large amount of work x:rep/large Large amount of reputation labels Feb 29, 2024

@mk-mxpmk-mxp left a comment

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.

The current solution using strings as the result is according to specification.

But I think, this is the perfect exercise to use an enum as a return type. That would mean, a) changing the implementation and b) pause the merging, until we have PHP8.1 as minimum version.

I would be very happy, if you agree to wait for that. When we merge this now, we cannot change the interface later to use enum as that invalidates all submitted solutions.

What do you think?

Comment threadexercises/practice/state-of-tic-tac-toe/StateOfTicTacToe.php Outdated
@mk-mxpmk-mxp added the paused Work paused until further notice label Feb 29, 2024
@tomasnorre

tomasnorre commented Feb 29, 2024

Copy link
Copy Markdown
ContributorAuthor

The current solution using strings as the result is according to specification.

But I think, this is the perfect exercise to use an enum as a return type. That would mean, a) changing the implementation and b) pause the merging, until we have PHP8.1 as minimum version.

I would be very happy, if you agree to wait for that. When we merge this now, we cannot change the interface later to use enum as that invalidates all submitted solutions.

What do you think?

I'm perfectly fine with enum and waiting. I think enum makes perfectly sense in this case.

Edit: Just for information, I have added this thread to the Forum
https://forum.exercism.org/t/php-versions-support/10070

@tomasnorre

Copy link
Copy Markdown
ContributorAuthor

It's expected to fail in PHP 8.0 as I have already added the enum we talked about yesterday. Then we might just need to rebase when we are ready to merge.

@mk-mxp

Copy link
Copy Markdown
Contributor

That's looks great! Looking forward to having a minimum of PHP 8.1...

@tomasnorre
tomasnorreforce-pushed the add-state-of-tic-tac-toe-exercise branch from 11122ae to 418fa17CompareMarch 4, 2024 14:02
@mk-mxp

Copy link
Copy Markdown
Contributor

@tomasnorre Today the PHP track got upgraded to the required PHP version. You may rebase this branch and update the PR.

@mk-mxpmk-mxp added x:type/content Work on content (e.g. exercises, concepts) and removed paused Work paused until further notice labels May 6, 2024
@tomasnorre
tomasnorreforce-pushed the add-state-of-tic-tac-toe-exercise branch from 3e54669 to f487a58CompareMay 7, 2024 19:44
@tomasnorre

tomasnorre commented May 7, 2024

Copy link
Copy Markdown
ContributorAuthor

I must have done something wrong in the rebasing. As all the changes are included.

I did

  1. Sync my fork
  2. git checkout branch
  3. git rebase main
  4. git push --force

Update: Looks better now. I did the sync of my fork incorrect.

@tomasnorre
tomasnorreforce-pushed the add-state-of-tic-tac-toe-exercise branch from f487a58 to 0f5c86dCompareMay 7, 2024 19:48

@mk-mxpmk-mxp left a comment

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.

Great! Thanks a lot.

@mk-mxp
mk-mxp merged commit d206439 into exercism:mainMay 8, 2024
@tomasnorre
tomasnorre deleted the add-state-of-tic-tac-toe-exercise branch May 8, 2024 14:08
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

x:action/createWork on something from scratchx:knowledge/elementaryLittle Exercism knowledge requiredx:module/practice-exerciseWork on Practice Exercisesx:rep/largeLarge amount of reputationx:size/largeLarge amount of workx:type/contentWork on content (e.g. exercises, concepts)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@tomasnorre@mk-mxp