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

Sync Acronym #769

Merged
merged 3 commits into from
Jul 7, 2024
Merged

Sync Acronym #769

merged 3 commits into from
Jul 7, 2024

Conversation

fejan-malek
Copy link
Contributor

#745

Below Changes done in this pull request

  • Updated the Markdown file.
  • New test cases added and removed tests which are not mentioned in test.toml

Copy link

This PR touches files which potentially affect the outcome of the tests of an exercise. This will cause all students' solutions to affected exercises to be re-tested.

If this PR does not affect the result of the test (or, for example, adds an edge case that is not worth rerunning all tests for), please add the following to the merge-commit message which will stops student's tests from re-running. Please copy-paste to avoid typos.

[no important files changed]

For more information, refer to the documentation. If you are unsure whether to add the message or not, please ping @exercism/maintainers-admin in a comment. Thank you!

@fejan-malek
Copy link
Contributor Author

@mk-mxp Should I update the example code here?

Please review thanks!

@mk-mxp
Copy link
Contributor

mk-mxp commented Jun 28, 2024

Please review thanks!

I hope to find time today. I'm busy with working for my living...

@fejan-malek
Copy link
Contributor Author

@mk-mxp

testUnderscoreEmphasis Is working correctly?

Should I also add in not included in test.toml? If yes then what is the reason because it running with the existing solution?

@mk-mxp
Copy link
Contributor

mk-mxp commented Jun 30, 2024

No underscores, please. Reasons:

The example.php is an internal solution, showing that

  • it is possible to solve the exercise in PHP
  • a solution exists, that runs fast enough for exercisms infrastructure

It is not enough, that example.php can handle the new tests. We need to look at the community solutions for very common solutions.

There are currently about 350 different community solutions (see https://exercism.org/tracks/php/build > "Practice Exercises" > 106 active exercises > Acronym). These are shown partially on the exercise community solutions page (see https://exercism.org/tracks/php/exercises/acronym/solutions when you have solved the exercise / are not logged in). Many of them use preg_*() functions and rely on the word boundary detection (\w and others) of PCRE. An underscore is not seen as a word boundary by PCRE, so these solutions fail.

@fejan-malek fejan-malek requested a review from mk-mxp July 2, 2024 08:59
Copy link
Contributor

@mk-mxp mk-mxp left a comment

Choose a reason for hiding this comment

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

Why haven't I seen this earlier? Shame on me...

exercises/practice/acronym/AcronymTest.php Outdated Show resolved Hide resolved
exercises/practice/acronym/AcronymTest.php Outdated Show resolved Hide resolved
@fejan-malek fejan-malek requested a review from mk-mxp July 6, 2024 06:24
Copy link
Contributor

@mk-mxp mk-mxp left a comment

Choose a reason for hiding this comment

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

Thank you for contribution!

@mk-mxp mk-mxp merged commit a1f3b20 into exercism:main Jul 7, 2024
12 checks passed
@mk-mxp mk-mxp added x:action/sync Sync content with its latest version x:knowledge/elementary Little Exercism knowledge required x:module/practice-exercise Work on Practice Exercises x:type/content Work on content (e.g. exercises, concepts) x:size/small Small amount of work x:rep/small Small amount of reputation labels Jul 7, 2024
@fejan-malek fejan-malek deleted the sync-acronym branch July 8, 2024 03:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
x:action/sync Sync content with its latest version x:knowledge/elementary Little Exercism knowledge required x:module/practice-exercise Work on Practice Exercises x:rep/small Small amount of reputation x:size/small Small amount of work x:type/content Work on content (e.g. exercises, concepts)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants