feat: add support for 'feedback --name slack-platform' #70

mwbrooks · 2025-04-29T22:37:52Z

CHANGELOG

Replaces feedback --name platform-improvements with feedback --name slack-platform as the flag name for Slack Platform support. You can continue to use --name platform-improvements until the next major version release.

Summary

This pull request replaces feedback --name platform-improvements with feedback --name slack-platform:

Deprecates feedback --name platform-improvements
Removes platform-improvements from the help documentation
Adds feedback --name slack-platform
Backwards compatible with --name platform-improvements continues to work until the next semver:major update

Preview

`slack feedback --help` displays `slack-platform`:

`slack feedback --name slack-platform`:

`slack feedback --name platform-improvements` is backwards compatible:

`slack feedback --name platform-improvements --verbose` deprecation message:

Reviewers

# Test Help
$ slack feedback --help
# → Confirm the option "slack-platform" exists
# → Confirm the option "platform-improvements" does not exist

$ slack feedback --name slack-platform
# → Should prompt to open the browser

$ slack feedback --name platform-improvements
# → Should prompt to open the browser

$ slack feedback --name platform-improvements --verbose
# → Confirm DEPRECATED message

Requirements

I've read and understood the Contributing Guidelines and have done my best effort to follow them.
I've read and agree to the Code of Conduct.

…ssage

codecov · 2025-04-29T22:41:58Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 63.18%. Comparing base (9bb1d69) to head (06e247e).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #70      +/-   ##
==========================================
+ Coverage   63.00%   63.18%   +0.17%     
==========================================
  Files         210      210              
  Lines       22183    22187       +4     
==========================================
+ Hits        13976    14018      +42     
+ Misses       7115     7082      -33     
+ Partials     1092     1087       -5

☔ 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

🚩 Some comments to guide the review and help answer a few open questions!

mwbrooks · 2025-04-29T22:39:58Z

cmd/feedback/feedback.go

+	SlackCLIFeedback                = "slack-cli"
+	SlackPlatformFeedback           = "slack-platform"
+	SlackPlatformFeedbackDeprecated = "platform-improvements" // DEPRECATED(semver:major)


note: Added a DEPRECATED(semver:X) comment to make this easier to find in the future. We are a little consistent with our deprecation comments - TODO(semver:major) and others also exist. Preference?

Thanks for both this comment and calling out the formatting!

IIRC we are using TODO(semver:major) more often, but this is often for internal breaking changes AFAICT.

I think DEPRECATED is meaningful here since it captures the ongoing support with an existing alternative option, but IMO the semver:major part is the most important! This is what I would use in searching for these 😉

Also I learned in review of #71 that go has a convention around making machine-readable comments that I'm interested in exploring:

...this comment directives are line comments that start with //go:, with no space between the // and the go:.

https://go.dev/wiki/Comments#directives

Using a similar pattern might be interesting for these more "conventional" comments, but I don't believe we have explicit notes on how these should be used overall...

One addition to this comment might also note the deprecated suggestions? 📚

Edit: I realize you make a similar comment a few lines down - no blocker here!

Suggested change

SlackCLIFeedback = "slack-cli"

SlackPlatformFeedback = "slack-platform"

SlackPlatformFeedbackDeprecated = "platform-improvements" // DEPRECATED(semver:major)

SlackCLIFeedback = "slack-cli"

SlackPlatformFeedback = "slack-platform"

SlackPlatformFeedbackDeprecated = "platform-improvements" // DEPRECATED(semver:major) - Replaced with the "slack-platform" option

The //go: directive is interesting. It led me down a path to find the use of the // DEPRECATED: comment in Golang. So, perhaps we can start to use that convention to signal deprecated code. I definitely agree that the (semver:major) scope is most important though!

mwbrooks · 2025-04-29T22:40:15Z

cmd/feedback/feedback.go

 			clients.IO.PrintInfo(ctx, false, fmt.Sprintf(
 				"%s\n%s\n",
-				style.Secondary("Ask questions, submit issues, or suggest features for the SLack CLI:"),
+				style.Secondary("Ask questions, submit issues, or suggest features for the Slack CLI:"),


note: Writing tests found a typo!

@mwbrooks Whew great catch! It's nice to know this wasn't released, but darn that's an L for sure 😉

mwbrooks · 2025-04-29T22:40:47Z

cmd/feedback/feedback.go

+	// DEPRECATED(semver:major): Support the deprecated survey name for backwards compatibility
+	if surveyNameFlag == SlackPlatformFeedbackDeprecated {
+		surveyNameFlag = SlackPlatformFeedback
+		clients.IO.PrintDebug(ctx, "DEPRECATED: The '--name %s' flag is deprecated; use '--name %s' instead", SlackPlatformFeedbackDeprecated, SlackPlatformFeedback)
+	}
+


note: I print a deprecation message to our PrintDebug instead of PrintWarning. We use both. Preference?

📣 I'm in favor of annoying and loud deprecation messages to encourage making expected changes as soon as possible.

This is a pattern I've found in build tools like goreleaser and nix that can be frustrating at first, but also save headache when the actual breaking changes do happen!

Cool with me! Commit 06e247e now prints this loud and in the open!

mwbrooks · 2025-04-29T22:41:43Z

cmd/feedback/feedback_test.go

+		_surveys := SurveyStore
 		SurveyStore = surveys
+		defer func() { SurveyStore = _surveys }()


note: After adding some Command Table Tests, I noticed that the tests started failing when ran together. The reason is that the current tests clobber the global SurveyStore. This code backs up and restores the original SurveyStore after each test.

🧪 In a follow-up PR, I'd like to refactor these tests to all use the Command Table Test.

zimeg

@mwbrooks The feedback command is coming together so nicely! It's much more intuitive to match the feedback target:

slack-cli: Slack CLI
slack-platform: Slack Platform
...

And I remain excited for what might else be included here 👾

A few rambles were made below, but this is a smooth experience already so please feel free to merge when the time is right 🚢 💨

zimeg · 2025-04-30T23:33:27Z

cmd/feedback/feedback.go

+	SlackCLIFeedback                = "slack-cli"
+	SlackPlatformFeedback           = "slack-platform"
+	SlackPlatformFeedbackDeprecated = "platform-improvements" // DEPRECATED(semver:major)


Thanks for both this comment and calling out the formatting!

IIRC we are using TODO(semver:major) more often, but this is often for internal breaking changes AFAICT.

I think DEPRECATED is meaningful here since it captures the ongoing support with an existing alternative option, but IMO the semver:major part is the most important! This is what I would use in searching for these 😉

zimeg · 2025-04-30T23:35:03Z

cmd/feedback/feedback.go

+	SlackCLIFeedback                = "slack-cli"
+	SlackPlatformFeedback           = "slack-platform"
+	SlackPlatformFeedbackDeprecated = "platform-improvements" // DEPRECATED(semver:major)


Also I learned in review of #71 that go has a convention around making machine-readable comments that I'm interested in exploring:

...this comment directives are line comments that start with //go:, with no space between the // and the go:.

https://go.dev/wiki/Comments#directives

Using a similar pattern might be interesting for these more "conventional" comments, but I don't believe we have explicit notes on how these should be used overall...

zimeg · 2025-04-30T23:37:00Z

cmd/feedback/feedback.go

+	SlackCLIFeedback                = "slack-cli"
+	SlackPlatformFeedback           = "slack-platform"
+	SlackPlatformFeedbackDeprecated = "platform-improvements" // DEPRECATED(semver:major)


One addition to this comment might also note the deprecated suggestions? 📚

Edit: I realize you make a similar comment a few lines down - no blocker here!

Suggested change

SlackCLIFeedback = "slack-cli"

SlackPlatformFeedback = "slack-platform"

SlackPlatformFeedbackDeprecated = "platform-improvements" // DEPRECATED(semver:major)

SlackCLIFeedback = "slack-cli"

SlackPlatformFeedback = "slack-platform"

SlackPlatformFeedbackDeprecated = "platform-improvements" // DEPRECATED(semver:major) - Replaced with the "slack-platform" option

zimeg · 2025-04-30T23:38:07Z

cmd/feedback/feedback.go

 			clients.IO.PrintInfo(ctx, false, fmt.Sprintf(
 				"%s\n%s\n",
-				style.Secondary("Ask questions, submit issues, or suggest features for the SLack CLI:"),
+				style.Secondary("Ask questions, submit issues, or suggest features for the Slack CLI:"),


@mwbrooks Whew great catch! It's nice to know this wasn't released, but darn that's an L for sure 😉

zimeg · 2025-04-30T23:40:50Z

cmd/feedback/feedback.go

+	// DEPRECATED(semver:major): Support the deprecated survey name for backwards compatibility
+	if surveyNameFlag == SlackPlatformFeedbackDeprecated {
+		surveyNameFlag = SlackPlatformFeedback
+		clients.IO.PrintDebug(ctx, "DEPRECATED: The '--name %s' flag is deprecated; use '--name %s' instead", SlackPlatformFeedbackDeprecated, SlackPlatformFeedback)
+	}
+


📣 I'm in favor of annoying and loud deprecation messages to encourage making expected changes as soon as possible.

This is a pattern I've found in build tools like goreleaser and nix that can be frustrating at first, but also save headache when the actual breaking changes do happen!

zimeg · 2025-04-30T23:45:51Z

cmd/feedback/feedback_test.go

+		"supports --name slack-platform": {
+			CmdArgs: []string{"--name", "slack-platform"},
+			Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) {
+				setupFeedbackCommandMocks(t, ctx, cm, cf)
+			},
+			ExpectedOutputs: []string{
+				"[email protected]",
+				"https://docs.slack.dev/developer-support",
+			},
+		},


mwbrooks · 2025-05-01T20:20:07Z

@zimeg Thanks again for the solid review and ideas around Golang comment directives! I'm going to follow-up with a PR that updates our maintainers guide with a recommendation to use the // DEPRECATED(semver:major): code comment 👌🏻

Before merging, I updated the deprecation warning to be printed to stdout! 🙉 💥 📣

mwbrooks added 15 commits April 24, 2025 11:19

feat: update 'feedback' command to include feedback about the Slack CLI

c0d8957

fix: update survey output to feedback output

a5348db

refactor: rename PlatformSurvey to SlackPlatformFeedback

e802c13

feat: support skipping query params for feedback urls

3afcc5a

test: add feedback name to trace

82a2d12

test: fix failing tests

50b499a

Merge branch 'main' into mwbrooks-feedback-github

4e705c1

chore: tweak comment

99a19e9

refactor: 'feedback' command always traces with slacktrace.FeedbackMe…

f373184

…ssage

refactor: add slackerrors for 'feedback' error handling

b635bca

refactor: clarify the ErrFeedbackNameRequired message

602085f

Merge branch 'main' into mwbrooks-feedback-github-p2

ef08e3c

feat: add support for 'feedback --name slack-platform'

3e6abf4

feat: add deprecation message

8bb4c1e

test: fix tests that clobber global

c745bda

mwbrooks added enhancement M-T: A feature request for new functionality changelog Use on updates to be included in the release notes semver:minor Use on pull requests to describe the release version increment labels Apr 29, 2025

mwbrooks added this to the Next Release milestone Apr 29, 2025

mwbrooks self-assigned this Apr 29, 2025

mwbrooks commented Apr 29, 2025

View reviewed changes

mwbrooks marked this pull request as ready for review April 29, 2025 22:42

mwbrooks requested a review from a team as a code owner April 29, 2025 22:42

zimeg approved these changes Apr 30, 2025

View reviewed changes

Base automatically changed from mwbrooks-feedback-github-p2 to main May 1, 2025 18:32

mwbrooks added 2 commits May 1, 2025 13:05

Merge branch 'main' into mwbrooks-feedback-github-p3

d3ea0a8

feat: print deprecation message to stdout

06e247e

mwbrooks merged commit 192ab46 into main May 1, 2025
6 checks passed

mwbrooks deleted the mwbrooks-feedback-github-p3 branch May 1, 2025 20:25

mwbrooks mentioned this pull request May 1, 2025

docs: add docs for deprecating a feature #74

Merged

2 tasks

feat: add support for 'feedback --name slack-platform' #70

feat: add support for 'feedback --name slack-platform' #70

Uh oh!

Conversation

mwbrooks commented Apr 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CHANGELOG

Summary

Preview

slack feedback --help displays slack-platform:

slack feedback --name slack-platform:

slack feedback --name platform-improvements is backwards compatible:

slack feedback --name platform-improvements --verbose deprecation message:

Reviewers

Requirements

Uh oh!

codecov bot commented Apr 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

mwbrooks left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zimeg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mwbrooks commented May 1, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mwbrooks commented Apr 29, 2025 •

edited

Loading

`slack feedback --help` displays `slack-platform`:

`slack feedback --name slack-platform`:

`slack feedback --name platform-improvements` is backwards compatible:

`slack feedback --name platform-improvements --verbose` deprecation message:

codecov bot commented Apr 29, 2025 •

edited

Loading