Skip to content

fix: disable upgrade notification for the manifest info command#548

Open
mwbrooks wants to merge 1 commit into
mainfrom
mwbrooks-update-ignore-manifest-info
Open

fix: disable upgrade notification for the manifest info command#548
mwbrooks wants to merge 1 commit into
mainfrom
mwbrooks-update-ignore-manifest-info

Conversation

@mwbrooks
Copy link
Copy Markdown
Member

@mwbrooks mwbrooks commented May 14, 2026

Changelog

Disable upgrade notification for the slack manifest info command. The command returns JSON that may be used for scripting and the upgrade notifications interfere with parsing the response output.

Summary

This pull request adds manifest info to the list of commands that suppress update notifications. The manifest info command outputs structured data consumed by other tools, so injecting an upgrade banner would corrupt the output.

Also extends isIgnoredCommand() to support multi-word subcommands (e.g., manifest info is ignored but manifest validate is not).

Preview

N/A

Testing

  1. Build with an artificially old version to trigger the upgrade notification:
    go build -ldflags="-X 'github.com/slackapi/slack-cli/internal/version.Version=v1.0.0'" -o bin/slack
  2. Delete the lastUpdateCheckedAt timestamp from ~/.slack/config.json so the check isn't skipped due to recency.
  3. Create a project:
    ./bin/slack create my-app/
    cd my-app/
  4. Verify slack manifest info does NOT show an upgrade notification:
    ./bin/slack manifest info --source local
  5. Verify another command DOES show the upgrade notification.
    ./bin/slack manifest validate

Requirements

@codecov
Copy link
Copy Markdown

codecov Bot commented May 14, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.66%. Comparing base (ebd9ae5) to head (f57f876).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #548      +/-   ##
==========================================
+ Coverage   71.64%   71.66%   +0.01%     
==========================================
  Files         225      225              
  Lines       19066    19068       +2     
==========================================
+ Hits        13660    13665       +5     
+ Misses       4202     4199       -3     
  Partials     1204     1204              

☔ 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.

@mwbrooks mwbrooks self-assigned this May 14, 2026
@mwbrooks mwbrooks added bug M-T: confirmed bug report. Issues are confirmed when the reproduction steps are documented changelog Use on updates to be included in the release notes semver:patch Use on pull requests to describe the release version increment labels May 14, 2026
@mwbrooks mwbrooks added this to the Next Release milestone May 14, 2026
@mwbrooks mwbrooks marked this pull request as ready for review May 14, 2026 23:44
@mwbrooks mwbrooks requested a review from a team as a code owner May 14, 2026 23:44
Copy link
Copy Markdown
Member

@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.

@mwbrooks LGTM! And thanks for saving confusion or error 🙏

I'm leaving a question that's alright to ignore and a praise for keeping code health improving constant! Please let's get this merged when time is right.

Comment thread internal/update/update.go
ignoredCommands := []string{"_fingerprint", "api", "version"}
osStr := os.Args[0:]
if len(osStr) < 2 {
ignoredCommands := []string{"_fingerprint", "api", "manifest info", "version"}
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.

Suggested change
ignoredCommands := []string{"_fingerprint", "api", "manifest info", "version"}
ignoredCommands := []string{"_fingerprint", "api", "manifest", "manifest info", "version"}

🪬 question: Is the shortened command automatic? I prefer extended commands often in scripts but am unsure if this should be covered too.

},
"manifest validate command": {
command: "manifest validate",
expected: false,
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.

👁️‍🗨️ praise: Thanks! This makes me think much about tests as spec more than unit.

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

Labels

bug M-T: confirmed bug report. Issues are confirmed when the reproduction steps are documented changelog Use on updates to be included in the release notes 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.

2 participants