-
-
Notifications
You must be signed in to change notification settings - Fork 219
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
UPDATE test cases from problem specifications for prime-factors #895
UPDATE test cases from problem specifications for prime-factors #895
Conversation
JagritGumber
commented
Aug 5, 2024
- Updated all the test cases including new ones added
- Regenerated test.toml with configlet
- Configlet test says all filepaths, metadata, and tests for prime-factors are up to date
- Updated all the test cases including new ones added - Regenerated test.toml with configlet - Configlet test says all filepaths, metadata, and tests for prime-factors are up to date
Hello. Thanks for opening a PR on Exercism 🙂 We ask that all changes to Exercism are discussed on our Community Forum before being opened on GitHub. To enforce this, we automatically close all PRs that are submitted. That doesn't mean your PR is rejected but that we want the initial discussion about it to happen on our forum where a wide range of key contributors across the Exercism ecosystem can weigh in. You can use this link to copy this into a new topic on the forum. If we decide the PR is appropriate, we'll reopen it and continue with it, so please don't delete your local branch. If you're interested in learning more about this auto-responder, please read this blog post. Note: If this PR has been pre-approved, please link back to this PR on the forum thread and a maintainer or staff member will reopen it. |
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.
For more information, refer to the documentation. If you are unsure whether to add the message or not, please ping |
Missed some semicolons and formatting will fix that right away |
This reverts commit 5224ab5.
According to the test.toml, the "large prime" test is not new. That might have been a mistake, as you are the one who included the test just now. I see three options:
I prefer option 3. Number 2 is "more of the same" as the other test. Do you have an opinion @siebenschlaefer @ErikSchierboom? |
I don't think that
I agree. |
That would touch ~1300 solutions. I could live with that, the fix is easy for everyone who understands the widening from int to long long. So if they want to fix it, it would not cost them a lot of time. |
Well if it is good with long long, I can work on updating the test cases to rather support |
You would also need to update the source and header files accordingly in the .meta directory. Is that okay? |
Yup, seems good! I see the examples use |
I'm fine with option 3, just as long as the PR contains |
Updated examples to also use long long instead of int [no important files changed]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good work. Thanks a lot. 🚀