Minor readme improvements #23
Conversation
Especially since an example for forget-results is given, people just skimming the documentation might inadvertenly pick up the wrong forget command.
inotifywait command copied from https://github.com/peff/git/blob/meta/ci mentioned in mhagger#6.
|
@mhagger Are there any blockers from merging this? |
mhagger
left a comment
mhagger
left a comment
There was a problem hiding this comment.
Thanks for your contribution. Sorry that I have neglected it.
I think that run-continuously.sh is a cool idea.
It looks like, when a test fails, git-test dumps its usual output to the screen, but the script continues running the test every 30 seconds. That surprised me at first; wouldn't one want the test to stop there so that the user can see what happened? But I suppose the idea is that the user continues developing in another window, so they might just want to fix the bug and see that the fix worked the next time that the test is run. So that seems fine.
It feels odd to have this script in the top-level directory, when even git-test is under bin. Though it also doesn't really belong in bin, since it has such a generic name.
What would you think of putting it in a directory called contrib and maybe giving it a slightly less generic name, like git-test-continuously? With the latter name, it could be installed then invoked as git test-continuously … which makes it more obvious that it's somehow related to git-test.
If I had energy to devote to this project, I'd suggest that the "test continuously" functionality could be an option added to git-test. But based on previous experience, you'd probably submit such a patch and I'd never get around to reviewing it 😢 so a separate script is probably better.
| * If commits are rewritten to change their log messages, authorship, dates, etc., the test results remain valid. | ||
| * If consecutive commits are squashed, the results remain valid. | ||
| * If a commit is split into two, only the first (partial) commit needs to be tested. | ||
| * If some commits deep in a branch are reordered, the test results for commits built on top of the reordered commits often remain valid. |
There was a problem hiding this comment.
Generally it's not welcome to change the formatting of documents in an existing project. The four-space indentation has advantages that I'd be happy to tell you about if you're curious, and surely two-space indentation does too. I don't think either one is more "correct", is it?
If that's true, I'd prefer for the whole first commit to be dropped, except for the addition of angle brackets around the URL (which is a correctness issue). But if this is too much trouble, I won't insist on it.
| inotifywait -qq -e delete_self -e move -r "$gitdir/HEAD" "$gitdir/refs" | ||
| else | ||
| n=0 | ||
| while [[ $n -lt ${GIT_TEST_CONTINOUS_INTERVAL:-30} ]] |
There was a problem hiding this comment.
There's a spelling error in several places in this PR: s/continously/continuously/. It's misspelled:
-
in the name of the script itself
-
in this environment variable name:
Suggested changewhile [[ $n -lt ${GIT_TEST_CONTINOUS_INTERVAL:-30} ]] while [[ $n -lt ${GIT_TEST_CONTINUOUS_INTERVAL:-30} ]] -
in the commit message for the commit that introduces this script
-
in the link and link text from
README.md
(I won't insist on the commit message being fixed, but if you're rewriting history anyway you might want to fix it, too.)
|
|
||
| * Dependencies between tests; for example: | ||
| * Continuous testing mode, where `git test` watches the repository for changes and re-runs itself automatically whenever the commits it is watching change. | ||
| For now you can use the [run-continously.sh](run-continously.sh) script. |
2 participants
No description provided.