This change makes the spellcheck test a failure instead of warning when it fails. See #25701 (comment).
I have also refactored the test to address MarcoFalke’s comments #24766#pullrequestreview-932953476.
This change makes the spellcheck test a failure instead of warning when it fails. See #25701 (comment).
I have also refactored the test to address MarcoFalke’s comments #24766#pullrequestreview-932953476.
ACK fefbca3509781740d606c1b43588c07aa0a36dae
0 #include <util/time.h>
1-
2+// this is a test coment
3 #include <deque>
4 #include <functional>
and getting the following error:
0src/torcontrol.cpp:21: coment ==> comment
1^ Error: codespell identified likely spelling errors. Any false positives? Add them to the list of ignored words in test/lint/spelling.ignore-words.txt
Thought about this a little more, I’m concept 0 on this.
Incorrect spelling of words in an inconsequential change shouldn’t lead to the CI being red.
What if your first language is not English, and you just care about writing good code? You publish a change, and CI keeps failing because of these darn English words; no one is leaving a review to fix your spelling or grammar. So your PR sits, where otherwise it might have been merged. This can be discouraging for new developers; especially of different backgrounds/native languages.
Now, in consideration of a really important change, and the comments written along with it, {insert criteria of important change here}, this would be more appropriate. But, we can’t selectively turn this on or off.
If we enable this, there should be a commitment to at least address the first scenario, if it ever is to come up. But, I’d rather avoid polluting PR’s with grammar nazism.
Concept ACK on the changes outside of making this a failure :)
@jarolrod I understand your point but isn’t it a bit extreme? Also, wouldn’t you already need to have a decent english level to start contributing to Bitcoin as most of the implementation documentation / code is written in english?
But, I’d rather avoid polluting PR’s with grammar nazism.
I understand but this change would also avoid pull requests that fix these mistakes.
It would be great to have a yellow/warning status for GitHub checks but unfortunately GitHub doesn’t provide this feature.
I’m happy to drop the exit(1)
if others feel the same.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
No conflicts as of last run.