Skip to content

Conversation

@zimeg
Copy link
Member

@zimeg zimeg commented Apr 18, 2025

Summary

This PR follows up from changes in #35 to check typos during tests.

Notes

Additional configuration is not included and this is instead left as a CI check to avoid drifting dependencies! 👾 ✨

I am of course interested in discussing this, but am hoping we can guard against regressions with these changes ❄️

Edit: Of course, also a kind thanks to @seratch for sharing this!

Requirements

@zimeg zimeg added semver:patch Use on pull requests to describe the release version increment build M-T: Changes to compilation and CI processes labels Apr 18, 2025
@zimeg zimeg added this to the Next Release milestone Apr 18, 2025
@zimeg zimeg self-assigned this Apr 18, 2025
@zimeg
Copy link
Member Author

zimeg commented Apr 18, 2025

🤔 Hmmm... The docs workflow is failing with these changes for some reason I have not seen:

Error: Not Found - https://docs.github.com/rest/repos/repos#get-a-repository

https://github.com/slackapi/slack-cli/actions/runs/14541228740/job/40799485080?pr=51#step:4:68

Edit: Related changes are in #52!

@codecov
Copy link

codecov bot commented Apr 18, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 62.90%. Comparing base (f3ba824) to head (434fb79).
Report is 73 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #51      +/-   ##
==========================================
- Coverage   62.91%   62.90%   -0.01%     
==========================================
  Files         210      210              
  Lines       22152    22152              
==========================================
- Hits        13936    13935       -1     
+ Misses       7132     7131       -1     
- Partials     1084     1086       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@zimeg
Copy link
Member Author

zimeg commented Apr 18, 2025

📣 Good news is that this check can cause CI to fail!

error: `UE` should be `USE`, `DUE`
  --> ./internal/prompts/app_select_test.go:4616:29
     |
4616 |  const enterprise1UserID = "UE1"
     |                             ^^
     |
Error: Process completed with exit code 2.

🔍 Bad news is that I tried to allow this pattern...

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

license-eye has checked 593 files.

Valid Invalid Ignored Fixed
462 1 130 0
Click to see the invalid file list
  • .typos.toml
Use this command to fix any missing license headers
```bash

docker run -it --rm -v $(pwd):/github/workspace apache/skywalking-eyes header fix

</details>

Copy link
Member Author

@zimeg zimeg left a comment

Choose a reason for hiding this comment

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

📝 A few notes on the worded and ignored changes!

I noticed with a configuration file some more words are found, which might not've been known otherwise. It makes for somewhat more predictable tests too IMO 👾

Comment on lines +3 to +4
"alle",
"groupe",
Copy link
Member Author

Choose a reason for hiding this comment

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

These are reserved constants not found in the included wordbanks:

var reserved = []string{
"alle",
"allgemein",
"aquí",
"canais",
"canal",
"channel",
"chaîne",
"conversation",
"eu",
"everyone",
"false",
"general",
"geral",
"group",
"groupe",

extend-ignore-identifiers-re = [
"alle",
"groupe",
"UE1",
Copy link
Member Author

Choose a reason for hiding this comment

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

This pattern appears as a testing placeholder sometimes!

const enterprise1UserID = "UE1"

{Command: "auth login", Meaning: "Login to a Slack account with prompts"},
{Command: "auth login --no-prompt", Meaning: "Login to a Slack account without prompts, this returns a ticket"},
{Command: "auth login --challenge 6d0a31c9 --ticket ISQWLiZT0OtMLO3YWNTJO0...", Meaning: "Complete login using ticket and challenge code"},
{Command: "auth login --challenge 6d0a31c9 --ticket ISQWLiZT0tOMLO3YWNTJO0...", Meaning: "Complete login using ticket and challenge code"},
Copy link
Member Author

Choose a reason for hiding this comment

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

The diff is not so clear, but "Ot" was replaced with "tO" to avoid unexpected identifiers found. It also reads more like "token" which is neat 🎉

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@zimeg
Copy link
Member Author

zimeg commented Apr 18, 2025

🔍 Here is a link to the spell checker passing tests:

https://github.com/slackapi/slack-cli/actions/runs/14543476644/job/40805487762?pr=51#step:4:37

$ ./typos .

@mwbrooks mwbrooks modified the milestones: v3.1.0, Next Release May 2, 2025
@mwbrooks mwbrooks modified the milestones: v3.2.0, Next Release May 16, 2025
@mwbrooks mwbrooks modified the milestones: v3.3.0, Next Release May 30, 2025
@mwbrooks mwbrooks modified the milestones: v3.4.0, Next Release Jun 13, 2025
@zimeg
Copy link
Member Author

zimeg commented Jun 17, 2025

📣 I realize an open issue in the slackapi/slack-health-score project to implement this might be ideal so I will be closing this PR now.

@zimeg zimeg closed this Jun 17, 2025
@zimeg zimeg deleted the zimeg-ci-typos branch June 17, 2025 22:42
@zimeg
Copy link
Member Author

zimeg commented Jun 17, 2025

📝 The issue mentioned above can be found here: slackapi/slack-health-score#99

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build M-T: Changes to compilation and CI processes semver:patch Use on pull requests to describe the release version increment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants