Skip to content

Use check-spelling/check-spelling@v0.0.20#9111

Merged
jekyllbot merged 1 commit intojekyll:masterfrom
jsoref:spell-check
Sep 23, 2022
Merged

Use check-spelling/check-spelling@v0.0.20#9111
jekyllbot merged 1 commit intojekyll:masterfrom
jsoref:spell-check

Conversation

@jsoref
Copy link
Copy Markdown
Contributor

@jsoref jsoref commented Aug 21, 2022

This is a 🐛 bug fix.

Summary

Context

Closes #9091

@mattr- mattr- requested a review from a team August 22, 2022 02:53
Copy link
Copy Markdown
Member

@ashmaroli ashmaroli left a comment

Choose a reason for hiding this comment

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

Thanks for improving the workflow, @jsoref. Left a few comments for further inputs.

outputs:
followup: ${{ steps.spelling.outputs.followup }}
runs-on: ubuntu-latest
if: "contains(github.event_name, 'pull_request') || github.event_name == 'push'"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Isn't this redundant since the top-level on key basically does the same..?

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.

Yes. This workflow is a subset of a version of the workflow which includes a comment handler.

It's fairly harmless to retain, but if you like, it can be dropped.

id: spelling
uses: check-spelling/check-spelling@v0.0.20
with:
suppress_push_for_open_pull_request: 1
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

These new lines need inline comments for future visitors.

needs: spelling
permissions:
contents: write
pull-requests: write
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we do away with PR comments? I think inline comments (check-run annotations) are more than sufficient.

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.

You only get 10 of each type (error, warning, notice) per job.

If you're ok with that constraint, then, yes.

Also note that if the lines that would be commented aren't changed in the PR (because they snuck into the base), then they usually won't be annotated. (GitHub occasionally shows such things, probably based on A/B testing or similar.)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes. 10 GitHub Annotations is sufficient. You may disable posting PR comments.

outputs:
followup: ${{ steps.spelling.outputs.followup }}
runs-on: ubuntu-latest
if: "contains(github.event_name, 'pull_request') || github.event_name == 'push'"
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.

Yes. This workflow is a subset of a version of the workflow which includes a comment handler.

It's fairly harmless to retain, but if you like, it can be dropped.

needs: spelling
permissions:
contents: write
pull-requests: write
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.

You only get 10 of each type (error, warning, notice) per job.

If you're ok with that constraint, then, yes.

Also note that if the lines that would be commented aren't changed in the PR (because they snuck into the base), then they usually won't be annotated. (GitHub occasionally shows such things, probably based on A/B testing or similar.)

Refreshes the workflow based on
https://github.com/check-spelling/spell-check-this/blob/744c66e2140fd8acaf5388efd0db3727d010d6e9/.github/workflows/spelling.yml

Changing the workflow not to post a comment, reporting is presented via
annotations.
Copy link
Copy Markdown
Member

@ashmaroli ashmaroli left a comment

Choose a reason for hiding this comment

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

LGTM!

@ashmaroli
Copy link
Copy Markdown
Member

Thanks @jsoref
@jekyllbot: merge +dev

@jekyllbot jekyllbot merged commit d1e392f into jekyll:master Sep 23, 2022
jekyllbot added a commit that referenced this pull request Sep 23, 2022
github-actions bot pushed a commit that referenced this pull request Sep 23, 2022
Josh Soref: Use check-spelling/check-spelling@v0.0.20 (#9111)

Merge pull request 9111
@jsoref jsoref deleted the spell-check branch September 23, 2022 17:43
@jekyll jekyll locked and limited conversation to collaborators Sep 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants