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

fix: delete the go formatter code as it is not longer used #3998

Merged
merged 1 commit into from
Sep 9, 2021

Conversation

skartikey
Copy link
Contributor

removed ast/format.go and ast/format_test.go
also removed test_rewriter package as its not longer in use

@skartikey skartikey requested review from a team and wolffcm and removed request for a team August 26, 2021 16:56
Copy link

@wolffcm wolffcm left a comment

Choose a reason for hiding this comment

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

Looks good, are you going to also make sure that the other repositories that use Flux can still build with the Go formatter removed?

@@ -13,6 +13,7 @@ import (

"github.com/influxdata/flux"
"github.com/influxdata/flux/ast"
"github.com/influxdata/flux/ast/astutil"
"github.com/influxdata/flux/ast/edit"
"github.com/influxdata/flux/codes"
"github.com/influxdata/flux/csv"
Copy link

Choose a reason for hiding this comment

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

As an aside, I don't think we need this code anymore... we should delete it.

Copy link

Choose a reason for hiding this comment

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

(I don't mean for you to delete as part of this PR)

@skartikey
Copy link
Contributor Author

Looks good, are you going to also make sure that the other repositories that use Flux can still build with the Go formatter removed?

I can see go formatter reference in idpe, kapacitor and query-archiver
In idpe, a big chunk of replacement has already been done by this PR https://github.com/influxdata/idpe/pull/10481
only a few occurrences are left.

I have sent a message on bulletins and will make sure all the references are replaced before merging this PR.

@danxmoran
Copy link
Contributor

Kapacitor switching over to the new formatter in influxdata/kapacitor#2618

@skartikey
Copy link
Contributor Author

Kapacitor switching over to the new formatter in influxdata/kapacitor#2618

@danxmoran I can still see 2 occurrences of it in the kapacitor repository. https://github.com/influxdata/kapacitor/search?q=ast.format Kindly have a look and let me know.

@danxmoran
Copy link
Contributor

The remaining occurrences of ast.Format in kapa are from a different ast package (for TICKscript)

@skartikey
Copy link
Contributor Author

Thanks @danxmoran, will merge this PR then.

removed ast/format.go and ast/format_test.go
also removed test_rewriter package as its not longer in use
@skartikey skartikey force-pushed the skartikey-flux-formatter branch from 9afdb96 to 552533b Compare September 9, 2021 15:46
@skartikey skartikey merged commit 93ce892 into master Sep 9, 2021
@skartikey skartikey deleted the skartikey-flux-formatter branch September 9, 2021 16:08
@skartikey skartikey linked an issue Sep 10, 2021 that may be closed by this pull request
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.

Delete ast/format.go and ast/format_test.go
3 participants