fix(analyze): honor offline mode for sharing#18
Conversation
|
@aqilaziz Can you rebase and fix Ci.. meanwhile im reviewing your PR |
|
@aqilaziz Btw Welcome! and Thanks for PR! |
|
Hehe, I’d recommend switching to Linux. Trust me, you’ll never regret it 😄 |
github-actions
Bot
added
documentation
|
Pushed follow-up fixes for the CI lint findings and resolved the merge conflict against current main without pulling unrelated workflow/history into this branch. Verification run locally:
GitHub now reports the PR as mergeable; waiting for the queued PR checks to finish. |
| return err | ||
| } | ||
| if shareFlag { | ||
| if shareFlag && offlineMode(offline) { |
There was a problem hiding this comment.
awkward as if A && B / else if A. nest it:
if shareFlag {
if offlineMode(offline) {
fmt.Fprintln(cmd.ErrOrStderr(), "warning: --share is ignored in --offline mode")
} else {
emitShareURL(cmd, rep)
}
}| return s == rules.SeverityHigh || s == rules.SeverityMed || s == rules.SeverityLow | ||
| } | ||
|
|
||
| func offlineMode(flag bool) bool { |
There was a problem hiding this comment.
worth a doc line saying this is the canonical egress gate. when watch or real audit add network calls they need to consult this too.
// offlineMode is the canonical egress gate. Any new network
// call in this binary MUST short-circuit when this returns true.
func offlineMode(flag bool) bool {| } | ||
| } | ||
|
|
||
| func TestAnalyze_OfflineShareWarnsAndSkipsUpload(t *testing.T) { |
There was a problem hiding this comment.
nothing here asserts --share actually fires emitShareURL when offline's off. add a sibling that captures stderr and greps for share URL:. that branch is now load-bearing.
| } | ||
| } | ||
|
|
||
| func TestAnalyze_OfflineEnvShareWarnsAndSkipsUpload(t *testing.T) { |
There was a problem hiding this comment.
right now nothing proves env beats an explicit --offline=false. add a variant with --offline=false --share and OPTIQOR_OFFLINE=1 set, assert the warning still fires.
Signed-off-by: aqilaziz <gonzes7@gmail.com>
|
Follow-up pushed in 0272f41 addressing the review items:
CI is green now. I also tried to update/rebase the branch against latest main, but GitHub blocks this token from carrying the upstream .github/workflows/ci.yml change because it lacks workflow scope. The PR is still mergeable, but it may need a maintainer-side update branch/rebase if your branch protection requires it. |
2 participants
Summary
--offlineandOPTIQOR_OFFLINE=1before running the--shareupload pathwarning: --share is ignored in --offline modeand exit successfully when sharing is skippedFixes #11
Tests
go test -count=1 ./cmd/optiqor -run 'TestAnalyze_(OfflineShareWarnsAndSkipsUpload|OfflineEnvShareWarnsAndSkipsUpload|HelpDocumentsOfflineEnv)$'go test -count=1 ./internal/... ./pkg/...git diff --checkNote:
make lint testcould not be run on this Windows machine becausemakeandgolangci-lintare not installed;go test -racealso requires cgo/gcc here. A fullgo test -count=1 ./...reaches existing golden-output mismatches caused by Windows path separators (<REPO>\...vs<REPO>/...).