-
Notifications
You must be signed in to change notification settings - Fork 454
fix: suppress osgetenvlibrary lint findings in pkg/cli and pkg/workflow #43967
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -87,7 +87,7 @@ func (c *AddInteractiveConfig) checkStatusAndOfferRun(ctx context.Context) error | |
| } | ||
|
|
||
| // In Codespaces, don't offer to trigger - provide link to Actions page instead | ||
| if os.Getenv("CODESPACES") == "true" { | ||
| if os.Getenv("CODESPACES") == "true" { //nolint:osgetenvlibrary | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [/codebase-design] 💡 Suggested refactor// Before
if os.Getenv("CODESPACES") == "true" { (nolint/redacted):osgetenvlibrary
// After
if isRunningInCodespace() {The existing @copilot please address this. |
||
| addInteractiveLog.Print("Running in Codespaces, skipping run offer and showing Actions link") | ||
| fmt.Fprintln(os.Stderr, "") | ||
| fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Running in GitHub Codespaces - please trigger the workflow manually from the Actions page")) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -84,7 +84,7 @@ Note: To create a new workflow from scratch, use the 'new' command instead.`, | |
|
|
||
| // add-wizard requires an interactive terminal | ||
| isTerminal := tty.IsStdoutTerminal() | ||
| isCIEnv := os.Getenv("CI") != "" | ||
| isCIEnv := os.Getenv("CI") != "" //nolint:osgetenvlibrary | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [/codebase-design] 💡 Suggested refactor// Before
isCIEnv := os.Getenv("CI") != "" (nolint/redacted):osgetenvlibrary
// After
isCIEnv := IsRunningInCI()@copilot please address this. |
||
| addWizardLog.Printf("Terminal check: is_terminal=%v, is_ci=%v", isTerminal, isCIEnv) | ||
| if !isTerminal || isCIEnv { | ||
| return errors.New("add-wizard requires an interactive terminal; use 'add' for non-interactive environments") | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -64,7 +64,7 @@ func CreateWorkflowInteractively(ctx context.Context, workflowName string, verbo | |
| interactiveLog.Printf("Starting interactive workflow creation: workflowName=%s, force=%v", workflowName, force) | ||
|
|
||
| // Assert this function is not running in automated unit tests | ||
| if os.Getenv("GO_TEST_MODE") == "true" || os.Getenv("CI") != "" { | ||
| if os.Getenv("GO_TEST_MODE") == "true" || os.Getenv("CI") != "" { //nolint:osgetenvlibrary | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [/codebase-design] Same pattern as 💡 Suggested refactor// Before
if os.Getenv("GO_TEST_MODE") == "true" || os.Getenv("CI") != "" { (nolint/redacted):osgetenvlibrary
// After
if os.Getenv("GO_TEST_MODE") == "true" || IsRunningInCI() {Both files guard against the same conditions; centralising removes the duplication. @copilot please address this. |
||
| return errors.New("interactive workflow creation cannot be used in automated tests or CI environments") | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[/codebase-design]
IsRunningInCI()already exists inci.go— using it here removes two direct env reads and their suppressions.💡 Suggested refactor
This also means future CI vars added to
IsRunningInCI()are automatically covered here.@copilot please address this.