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

add auto goimports into Makefile #182

Merged
merged 7 commits into from Feb 3, 2020
Merged

Conversation

YangKeao
Copy link
Member

@YangKeao YangKeao commented Feb 3, 2020

Signed-off-by: Yang Keao keao.yang@yahoo.com

What problem does this PR solve?

Add goimports command into Makefile.

What is changed and how does it work?

Diff check has been added to CI to ensure every PR has been formatted and yaml has been regenerated.

Tests

  • No code

Signed-off-by: Yang Keao <keao.yang@yahoo.com>
@YangKeao YangKeao requested review from fewdan and cwen0 February 3, 2020 07:10
Makefile Outdated
$(GO) fmt ./...

groupimports:
Copy link
Contributor

Choose a reason for hiding this comment

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

Did we need to install goimport first

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes! Though goimports is auto installed as golang.org/x/tools is an indirect dependency, it's still necessary to check and install.

@Yisaer
Copy link
Contributor

Yisaer commented Feb 3, 2020

We should also add this into the CI test.

Copy link
Contributor

@zhouqiang-cl zhouqiang-cl left a comment

Choose a reason for hiding this comment

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

LGTM

@codecov-io
Copy link

codecov-io commented Feb 3, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@77e897e). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #182   +/-   ##
=========================================
  Coverage          ?   39.47%           
=========================================
  Files             ?       17           
  Lines             ?      608           
  Branches          ?        0           
=========================================
  Hits              ?      240           
  Misses            ?      337           
  Partials          ?       31

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 77e897e...0dbe18a. Read the comment docs.

@zhouqiang-cl
Copy link
Contributor

/test

1 similar comment
@YangKeao
Copy link
Member Author

YangKeao commented Feb 3, 2020

/test

Copy link
Member

@you06 you06 left a comment

Choose a reason for hiding this comment

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

LGTM

@zhouqiang-cl zhouqiang-cl merged commit ea2fa91 into chaos-mesh:master Feb 3, 2020
@YangKeao YangKeao deleted the tune_ci branch February 3, 2020 09:10
sjwsl pushed a commit to sjwsl/chaos-mesh that referenced this pull request May 6, 2021
* add auto goimports into Makefile

Signed-off-by: Yang Keao <keao.yang@yahoo.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants