Skip to content
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

CI: Add Typos workflow #7475

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

szepeviktor
Copy link
Contributor

@szepeviktor szepeviktor commented Nov 26, 2023

@szepeviktor szepeviktor changed the title Create spelling.yml chore: Add Typos workflow Nov 26, 2023
@szepeviktor szepeviktor marked this pull request as ready for review November 26, 2023 12:54
Comment on lines +7 to +8
permissions:
contents: read
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually the read permission is not needed for public repos.
https://github.com/szepeviktor/byte-level-care/blob/master/.github/workflows/spelling.yml#L13

Copy link
Member

Choose a reason for hiding this comment

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

not sure if it is still the case, but there was a moment when if no permissions declared explicitly, some more than "read" was allowed.

explicit > implicit

.typos.toml Show resolved Hide resolved

[default]
extend-ignore-re = [
"Sebastiaan Stok",
Copy link
Member

Choose a reason for hiding this comment

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

Instead of listing Sebastiaan explicitly, maybe we can use @author .* and just ignore any contributor's name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

$tag = new Annotation([new Line(' * @author Chuck Norris')]);

I want to make sure Chuck Norris' name is never misspelled 🙃

Copy link
Member

@Wirone Wirone Nov 26, 2023

Choose a reason for hiding this comment

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

I just wouldn't want to maintain this file everytime new contributor provides new files with author that does not fit typo checker, especially there's no official way of running it locally (it's not a part of composer qa).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

typos is written in Go and is a single binary 1️⃣

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How many incidents will we have? One typo-like named contributor every 5 years?

Copy link
Member

Choose a reason for hiding this comment

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

typos is written in Go and is a single binary 1️⃣

So maybe we should install a binary file in a Composer hook (post-autoload-dump) and allow running it locally? composer typos added to composer static-analysis would allow preventing failures in CI.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know how a developer's life goes.

Copy link
Member

Choose a reason for hiding this comment

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

agree, let's treat it similar as checkbashisms

.typos.toml Show resolved Hide resolved
@Wirone Wirone requested a review from keradus November 26, 2023 18:21
@Wirone Wirone changed the title chore: Add Typos workflow CI: Add Typos workflow Nov 26, 2023
@coveralls
Copy link

coveralls commented Nov 26, 2023

Coverage Status

coverage: 94.918% (-0.004%) from 94.922%
when pulling ce95951 on szepeviktor:patch-1
into 39596a3 on PHP-CS-Fixer:master.

- name: Checkout repository
uses: actions/checkout@v4
- name: Search for misspellings
uses: crate-ci/typos@master
Copy link
Member

Choose a reason for hiding this comment

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

any semver possible?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, the author of typos does not tag it as - for example - "v2", only semver tags exist but typos is improving so fast you don't want to use a semver tag.

This comment was marked as outdated.

@Wirone Wirone added status/rebase required PR is outdated and required synchronisation with main branch and removed status/stale labels Mar 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/rebase required PR is outdated and required synchronisation with main branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants