Skip to content
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

Cleanup: Remove the ai-logs flag. #679

Merged
merged 2 commits into from
Oct 4, 2024
Merged

Cleanup: Remove the ai-logs flag. #679

merged 2 commits into from
Oct 4, 2024

Conversation

jlewi
Copy link
Contributor

@jlewi jlewi commented Oct 2, 2024

@sourishkrout
Copy link
Member

You doing push & pray over there 😆, @jlewi?

@jlewi
Copy link
Contributor Author

jlewi commented Oct 4, 2024

You caught me.

@jlewi
Copy link
Contributor Author

jlewi commented Oct 4, 2024

Lets try this again.

I fixed the build.

make build                       
CGO_ENABLED=0 go build -o runme -ldflags="-s -w -X 'github.com/stateful/runme/v3/internal/version.BuildDate=2024-10-04T00:33:55Z' -X 'github.com/stateful/runme/v3/internal/version.BuildVersion=3.8.3-2-ge50ed70-e50ed70' -X 'github.com/stateful/runme/v3/internal/version.Commit=e50ed7058be0f61a206168fd3467886e7202034e'" main.go

Lint gives me errors when running locally. But errors seem unrelated to any files I modified.

jlewi@~/git_runme#make lint
diff cmd/gqltool/main.go.orig cmd/gqltool/main.go
--- cmd/gqltool/main.go.orig
+++ cmd/gqltool/main.go
@@ -11,10 +11,9 @@
 	"github.com/stateful/runme/v3/internal/client/graphql"
 )
 
-var (
-	apiURL = flag.String("api-url", "http://localhost:4000", "The API base address")
-	// tokenDir = flag.String("token-dir", cmd.GetUserConfigHome(), "The directory with tokens")
-)
+var apiURL = flag.String("api-url", "http://localhost:4000", "The API base address")
+
+// tokenDir = flag.String("token-dir", cmd.GetUserConfigHome(), "The directory with tokens")
 
 func init() {
 	flag.Parse()
diff internal/runner/service.go.orig internal/runner/service.go
--- internal/runner/service.go.orig
+++ internal/runner/service.go
@@ -682,9 +682,7 @@
 		return nil, err
 	}
 
-	var (
-		modifiedScriptBuf bytes.Buffer
-	)
+	var modifiedScriptBuf bytes.Buffer
 
 	script := req.GetScript()
 	if commands := req.GetCommands(); script == "" && len(commands.Lines) > 0 {
diff internal/runnerv2service/service_resolve_program.go.orig internal/runnerv2service/service_resolve_program.go
--- internal/runnerv2service/service_resolve_program.go.orig
+++ internal/runnerv2service/service_resolve_program.go
@@ -26,9 +26,7 @@
 		return nil, err
 	}
 
-	var (
-		modifiedScriptBuf bytes.Buffer
-	)
+	var modifiedScriptBuf bytes.Buffer
 
 	script := req.GetScript()
 	if commands := req.GetCommands(); script == "" && len(commands.Lines) > 0 {
diff internal/telemetry/scarf.go.orig internal/telemetry/scarf.go
--- internal/telemetry/scarf.go.orig
+++ internal/telemetry/scarf.go
@@ -19,8 +19,10 @@
 	client = "Kernel"
 )
 
-type reporterFunc func() error
-type lookupEnvFunc func(key string) (string, bool)
+type (
+	reporterFunc  func() error
+	lookupEnvFunc func(key string) (string, bool)
+)
 
 var reporter reporterFunc

Also seeing some test failures locally.

    --- FAIL: TestRunnerServiceServerExecute_WithInput/SimulateCtrlC (1.51s)
        service_execute_test.go:842: 
            	Error Trace:	/Users/jlewi/git_runme/internal/runnerv2service/service_execute_test.go:842
            	Error:      	"rpc error: code = Unknown desc = exit status 1" does not contain "exit status 130"
            	Test:       	TestRunnerServiceServerExecute_WithInput/SimulateCtrlC
        service_execute_test.go:843: 
            	Error Trace:	/Users/jlewi/git_runme/internal/runnerv2service/service_execute_test.go:843
            	Error:      	Not equal: 
            	            	expected: 130
            	            	actual  : 1
            	Test:       	TestRunnerServiceServerExecute_WithInput/SimulateCtrlC

Looks in CI there is a test timeout now. Are tests known to be flaky?

* This flag is deprecated and no longer used.
* The frontend should have stopped using this flag starting in 3.7.5
* The frontend is now up to 3.8.5
* So we should be good to remove this flag because the frontend should no longer be setting it.

* Related to jlewi/foyle#211
Copy link

sonarcloud bot commented Oct 4, 2024

Copy link
Member

@sourishkrout sourishkrout left a comment

Choose a reason for hiding this comment

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

✅ LGTM. I have an eye on the flakes. I made them more stable on Linux but they still only seem to flake in GHA :-/.

@sourishkrout sourishkrout merged commit 03e23d3 into main Oct 4, 2024
7 checks passed
@sourishkrout sourishkrout deleted the jlewi/flagcleanup branch October 4, 2024 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants