-
Notifications
You must be signed in to change notification settings - Fork 520
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
DO NOT MERGE: Examples for #386 #387
Closed
coriolinus
wants to merge
11
commits into
exercism:master
from
coriolinus:DO-NOT-MERGE-example-for-386
Closed
DO NOT MERGE: Examples for #386 #387
coriolinus
wants to merge
11
commits into
exercism:master
from
coriolinus:DO-NOT-MERGE-example-for-386
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Checking readmes takes up non-trivial maintainer time, and is prone to error. This script adds a check to the CI which ensures that PRs are only accepted if their README.md files are present and correct. It begins by isolating the exercises already impacted by the PR, so that upstream changes to unrelated exercises don't cause undue difficulty. It then simply ensures that the README.md within the PR is identical to the one generated by `configlet generate`. Outputs lists of exercises with missing or incorrect READMEs, so people for whom CI fails due to this test not passing should have an easy time understanding what went wrong and how to fix it.
Checking readmes takes up non-trivial maintainer time, and is prone to error. This script adds a check to the CI which ensures that PRs are only accepted if their README.md files are present and correct. It begins by isolating the exercises already impacted by the PR, so that upstream changes to unrelated exercises don't cause undue difficulty. It then simply ensures that the README.md within the PR is identical to the one generated by `configlet generate`. Outputs lists of exercises with missing or incorrect READMEs, so people for whom CI fails due to this test not passing should have an easy time understanding what went wrong and how to fix it.
This commit is designed to cause Travis to fail, if exercism#386 is merged and implemented correctly. It should check the following exercises, modified by this PR: all-your-base crypto-square decimal hello-world rectangles two-bucket The following two exercises should be reported as missing README.md: all-your-base decimal The followign two exercises should be reported as having out of date READMEs.md: crypto-square hello-world The following two exercises should be checked, but pass without further output: rectangles two-bucket Note that if Travis runs on these without including the `_test/ensure-readmes-are-updated.sh` test, it will likely pass without comment.
Looks like this did fail, though for the wrong reasons: The command "sh ./_test/ensure-stubs-compile.sh" exited with 0.
0.44s$ sh ./_test/count-ignores.sh
/home/travis/build/exercism/rust/exercises/rectangles/tests/more_tests.rs: Has 0 tests and 0 ignores (should be -1)
/home/travis/build/exercism/rust/exercises/two-bucket/tests/more_tests.rs: Has 0 tests and 0 ignores (should be -1)
The command "sh ./_test/count-ignores.sh" exited with 1.
0.32s$ ./bin/fetch-configlet
The command "./bin/fetch-configlet" exited with 0.
0.04s$ sh ./_test/ensure-readmes-are-updated.sh
Checking readme for all-your-base
Checking readme for crypto-square
-> path not found: /home/travis/build/exercism/problem-specifications
Checking readme for decimal
Checking readme for hello-world
-> path not found: /home/travis/build/exercism/problem-specifications
Checking readme for rectangles
-> path not found: /home/travis/build/exercism/problem-specifications
Checking readme for two-bucket
-> path not found: /home/travis/build/exercism/problem-specifications
Exercises missing README.md:$
all-your-base$
decimal
The command "sh ./_test/ensure-readmes-are-updated.sh" exited with 1. More work to the script and the test cases will be required. |
…ready exist This enables us to point configlet at a known location where it should exist, which in turn should make Travis happier with this.
…ercism_rust into ensure-readmes-are-updated Conflicts: _test/ensure-readmes-are-updated.sh
This commit is designed to cause Travis to fail, if exercism#386 is merged and implemented correctly. It should check the following exercises, modified by this PR: all-your-base crypto-square decimal hello-world rectangles two-bucket The following two exercises should be reported as missing README.md: all-your-base decimal The followign two exercises should be reported as having out of date READMEs.md: crypto-square hello-world The following two exercises should be checked, but pass without further output: rectangles two-bucket Note that if Travis runs on these without including the `_test/ensure-readmes-are-updated.sh` test, it will likely pass without comment.
…exercism_rust into DO-NOT-MERGE-example-for-386
Travis now failed for kind of the right reasons: $ sh ./_test/ensure-readmes-are-updated.sh
Cloning into 'problem-specifications'...
remote: Counting objects: 6150, done.
remote: Total 6150 (delta 0), reused 0 (delta 0), pack-reused 6150
Receiving objects: 100% (6150/6150), 1.20 MiB | 18.38 MiB/s, done.
Resolving deltas: 100% (3348/3348), done.
Checking readme for all-your-base
Checking readme for crypto-square
Checking readme for decimal
Checking readme for hello-world
Checking readme for rectangles
Checking readme for two-bucket
Exercises missing README.md:$
all-your-base$
decimal
Exercises with out-of-date README.md:$
crypto-square$
hello-world$
rectangles$
two-bucket
The command "sh ./_test/ensure-readmes-are-updated.sh" exited with 1. Looks good, but |
This now fails correctly: $ sh ./_test/ensure-readmes-are-updated.sh
Cloning into 'problem-specifications'...
remote: Counting objects: 6155, done.
remote: Compressing objects: 100% (5/5), done.
remote: Total 6155 (delta 0), reused 1 (delta 0), pack-reused 6150
Receiving objects: 100% (6155/6155), 1.21 MiB | 15.45 MiB/s, done.
Resolving deltas: 100% (3348/3348), done.
Checking readme for all-your-base
Checking readme for crypto-square
Checking readme for decimal
Checking readme for hello-world
Checking readme for rectangles
Checking readme for two-bucket
Exercises missing README.md:$
all-your-base$
decimal
Exercises with out-of-date README.md:$
crypto-square$
hello-world
The command "sh ./_test/ensure-readmes-are-updated.sh" exited with 1. Now that this works, #386 should be able to proceed. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit is designed to cause Travis to fail, if #386 is merged
and implemented correctly.
It should check the following exercises, modified by this PR:
The following two exercises should be reported as missing README.md:
The following two exercises should be reported as having out of date READMEs.md:
The following two exercises should be checked, but pass without further output:
If Travis approves this PR, something has gone wrong.