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

Fix: Various spellchecks #835

Merged
merged 2 commits into from
Nov 5, 2024
Merged

Conversation

astyanax
Copy link
Contributor

@astyanax astyanax commented Oct 31, 2024

I used various automated tools to determine mostly orthographic errors, but I edited whatever errors I could find.
I manually fixed everything (no automated edits).
I'm thinking that the "translater" change was an overkill, but I wanted to be pedantic.
edit: I added the "[no important files changed]" bit in the commit message

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!

[no important files changed]
@tomasnorre
Copy link
Contributor

tomasnorre commented Oct 31, 2024

Thanks for you PR. As a lot of the spelling that you have correct come from https://github.com/exercism/problem-specifications , it would be great if you could create a PR against, that repository too.

This will make sure that all tracks on exercism.org will profit from this change next time their exercises gets synced.

This PR changes the API of some exercises, which will result in already completed exercises will fail. Therefor this can in my opinion not be merged, sorry.

@mk-mxp, @neenjaw or @homersimpsons please way in here.

@homersimpsons
Copy link
Contributor

This PR changes the API of some exercises, which will result in already completed exercises will fail. Therefor this can in my opinion not be merged, sorry.

@mk-mxp, @neenjaw or @homersimpsons please way in here.

Looking at the changes it seems that only the ProteinTranslationTest.php could impact the results (others are markdown files). But reading the change it is just a renaming of a class variable used within the test. This should have no effect on test outcome.

(I'm on mobile so maybe I didn't see another change)

About the suggestion to update problem-specifications repository I agree with that if those typos are there too.

@tomasnorre
Copy link
Contributor

Looking at the changes it seems that only the ProteinTranslationTest.php could impact the results (others are markdown files). But reading the change it is just a renaming of a class variable used within the test. This should have no effect on test outcome.

I think you're right, I overlooked that the only changes done was in PHP was the test file. Shouldn't break anything.

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.

Thanks a lot for these corrections! Just one small change required, where Search&Replace seems to have gone too far.

@mk-mxp mk-mxp added x:action/fix Fix an issue x:knowledge/none No existing Exercism knowledge required x:module/concept Work on Concepts 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 Nov 1, 2024
Co-authored-by: mk-mxp <55182845+mk-mxp@users.noreply.github.com>
@astyanax
Copy link
Contributor Author

astyanax commented Nov 5, 2024

Thank you all for your comments and careful consideration, and thank you Tomas for the patch!

p.s. It'll take some time to grok problem-specifications and how configlet interacts with it, #track-data-tooling implies that corrections would need to be explicitly applied/overridden.

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.

Thanks everyone for contributing!

@mk-mxp mk-mxp merged commit 425903c into exercism:main Nov 5, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
x:action/fix Fix an issue x:knowledge/none No existing Exercism knowledge required x:module/concept Work on Concepts 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.

5 participants