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

[no-release-notes] fix memo gen #1428

Merged
merged 2 commits into from
Nov 29, 2022
Merged

Conversation

max-hoffman
Copy link
Contributor

No description provided.

@PavelSafronov
Copy link
Contributor

Running go generate ./... on this branch creates 3 changed files. I think those should be included in this PR.

@max-hoffman
Copy link
Contributor Author

i see one import format change in sql/analyzer/memo.og.go. what do you see?

@max-hoffman
Copy link
Contributor Author

note the generator is poorly isolated from the source files. i have been slowly fixing these, but currently you have to install to regen.

@PavelSafronov
Copy link
Contributor

I'm seeing these changes:
image

@max-hoffman
Copy link
Contributor Author

max-hoffman commented Nov 29, 2022

Our CI enforces a strict but auto-committed import formatting that I have not been able to perfectly replicate with the goimports API.

Explicitly, we run:

goimports -w -local github.com/dolthub/go-mysql-server .

Which I run in Go as:

b, err = imports.Process("github.com/dolthub/go-mysql-server", buf.Bytes(), nil)

This is the remaining difference:

--- a/sql/parse/window_frame_factory.og.go
+++ b/sql/parse/window_frame_factory.og.go
@@ -5,10 +5,9 @@ package parse
 import (
        "fmt"

-       ast "github.com/dolthub/vitess/go/vt/sqlparser"
-
        "github.com/dolthub/go-mysql-server/sql"
        "github.com/dolthub/go-mysql-server/sql/plan"
+       ast "github.com/dolthub/vitess/go/vt/sqlparser"
 )

Punting for now in favor of customer bugs.

@max-hoffman max-hoffman merged commit 012637c into main Nov 29, 2022
@max-hoffman max-hoffman deleted the max/fix-memo-gen-table-func branch November 29, 2022 18:13
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