Skip to content
This repository was archived by the owner on Jun 28, 2024. It is now read-only.

ci: static check: spellcheck: check all docs#2228

Merged
GabyCT merged 1 commit intokata-containers:masterfrom
grahamwhaley:20200116_spellcheck_link
Jan 16, 2020
Merged

ci: static check: spellcheck: check all docs#2228
GabyCT merged 1 commit intokata-containers:masterfrom
grahamwhaley:20200116_spellcheck_link

Conversation

@grahamwhaley
Copy link
Copy Markdown
Contributor

Rather than fail the spell check on the first failure, continue to
check all the changed documents, and report the global fail at the
end. This may help reduce CI/review cycle time if/when multiple docs
are submitted in a single PR.

Also spit out a URL link to the documentation repo docs in the case
of failure, to help direct authors to guidance docs.

Fixes: #2227

Signed-off-by: Graham Whaley graham.whaley@intel.com

@grahamwhaley
Copy link
Copy Markdown
Contributor Author

@jodh-intel @chavafg - can we stare hard at this bash - it is slightly tricky, and I can't easily test it locally, and I think the CI won't really test it - so we might need some 👀 all over it..

"$cmd" check "$doc" || { info "spell check failed for document $doc" && docs_failed=1; }
done

[ $docs_failed -ne 0 ] || die "spell check failed, See https://github.com/kata-containers/documentation/blob/master/Documentation-Requirements.md#spelling for more information."
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be && instead of ||
or
use -eq instead of -ne

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, yup, I think you are right. that is a copy/paste/do-not-engage-brain moment :-)

Rather than fail the spell check on the first failure, continue to
check all the changed documents, and report the global fail at the
end. This may help reduce CI/review cycle time if/when multiple docs
are submitted in a single PR.

Also spit out a URL link to the documentation repo docs in the case
of failure, to help direct authors to guidance docs.

Fixes: kata-containers#2227

Signed-off-by: Graham Whaley <graham.whaley@intel.com>
@grahamwhaley grahamwhaley force-pushed the 20200116_spellcheck_link branch from 7b719c4 to c4a2919 Compare January 16, 2020 13:42
for doc in $docs
do
"$cmd" check "$doc" || die "spell check failed for document $doc"
"$cmd" check "$doc" || { info "spell check failed for document $doc" && docs_failed=1; }
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could increment the variable value if we want to tell the user how many errors he got, but if not, I think this way is fine.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did originally think of increment, but then ran away from the idea of adding any bc or (( )) code into a single line expression, so went with the simple =1 option. We could also collect up the names of the $docs that failed, and list them at the end.

I can add those improvements if we want. I think as it is, all the necessary info should appear in the logs fairly grouped together.
Somebody just shout if they want the improvements adding.

@chavafg
Copy link
Copy Markdown
Contributor

chavafg commented Jan 16, 2020

/test-ubuntu

@GabyCT GabyCT merged commit 0045778 into kata-containers:master Jan 16, 2020
grahamwhaley pushed a commit to grahamwhaley/kata-containers-tests that referenced this pull request Jan 20, 2020
During the merge of kata-containers#2228 the final check in the spell check got
inverted - fix with a `|| true`.

Fixes: kata-containers#2239

Signed-off-by: Graham Whaley <graham.whaley@intel.com>
grahamwhaley pushed a commit to grahamwhaley/kata-containers-tests that referenced this pull request Jan 20, 2020
During the merge of kata-containers#2228 the final check in the spell check got
inverted - fix with a nice test inversion.

Fixes: kata-containers#2239

Signed-off-by: Graham Whaley <graham.whaley@intel.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ci: spellcheck: don't stop checking on first doc failure

4 participants