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

feat(build.sh): Enable goimports + cleanup imports in source files #186

Merged
merged 3 commits into from
Jun 28, 2019

Conversation

rhuss
Copy link
Contributor

@rhuss rhuss commented Jun 14, 2019

Much like 'go fmt' for cleaning up go source, 'goimports' can help
in properly formatting and grouping imports. See https://goinbigdata.com/goimports-vs-gofmt/
for a quick overview.

This commit also cleansup imports on existing files.

@googlebot googlebot added the cla: yes Indicates the PR's author has signed the CLA. label Jun 14, 2019
@knative-prow-robot knative-prow-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jun 14, 2019
hack/build.sh Outdated
@@ -61,6 +61,9 @@ run() {
# Format source code
go_fmt

# Cleanup imports
go_imports
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor... same as comment to go_fmt. I'd move anything that changes the code before compile and test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will change the order in another PR, just let those get merged first.

hack/build.sh Outdated
@@ -61,6 +61,10 @@ run() {
# Format source code
go_fmt
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we remove the go_fmt now? since, in addition to fixing imports, goimports also formats code in the same style as gofmt.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah, ok. I thought they do different things. Let me check.

@rhuss rhuss changed the title feat(build.sh): Enable goimports for cleanup imports + cleanup imports feat(build.sh): Enable goimports + cleanup imports in source files Jun 18, 2019
@mattmoor mattmoor removed their request for review June 21, 2019 03:15
@sixolet
Copy link
Contributor

sixolet commented Jun 25, 2019

/approve
/lgtm

@knative-prow-robot knative-prow-robot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jun 25, 2019
rhuss added 2 commits June 28, 2019 13:35
Much like 'go fmt' for cleaning up go source, 'goimports' can help
in properly formatting and grouping imports. See https://goinbigdata.com/goimports-vs-gofmt/
for a quick overview.

This commit also cleansup imports on existing files.
@knative-prow-robot knative-prow-robot removed the lgtm Indicates that a PR is ready to be merged. label Jun 28, 2019
@knative-prow-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rhuss, sixolet

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

And changed order so that formatting and import reordering comes
before compiling and testing
@maximilien
Copy link
Contributor

/lgtm

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 28, 2019
@knative-prow-robot knative-prow-robot merged commit a60d273 into knative:master Jun 28, 2019
maximilien pushed a commit to maximilien/client that referenced this pull request Jul 1, 2019
…native#186)

* feat(build.sh): Enable goimports for cleanup imports + cleanup imports

Much like 'go fmt' for cleaning up go source, 'goimports' can help
in properly formatting and grouping imports. See https://goinbigdata.com/goimports-vs-gofmt/
for a quick overview.

This commit also cleansup imports on existing files.

* fix(build): Add missing space for output on iterm

* chore(build): Merged gofmt and goimports

And changed order so that formatting and import reordering comes
before compiling and testing
@rhuss rhuss deleted the pr/goimports branch March 9, 2021 10:32
dsimansk added a commit to dsimansk/client that referenced this pull request Jun 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cla: yes Indicates the PR's author has signed the CLA. lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants