Skip to content
This repository was archived by the owner on Oct 24, 2023. It is now read-only.

fix: address goimports checks#204

Merged
mboersma merged 1 commit intoAzure:masterfrom
sylr:goimports
Jan 11, 2019
Merged

fix: address goimports checks#204
mboersma merged 1 commit intoAzure:masterfrom
sylr:goimports

Conversation

@sylr
Copy link
Contributor

@sylr sylr commented Dec 26, 2018

/home/sylvain/gopath/bin/goimports -w -l -srcdir ~/gopath/src/github.com/Azure/aks-engine main.go $(find . -type d ! -path ./vendor\* ! -path ./.git\* ! -path .)

@acs-bot acs-bot added the size/M label Dec 26, 2018
@acs-bot
Copy link

acs-bot commented Dec 26, 2018

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: sylr
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: mboersma

If they are not already assigned, you can assign the PR to them by writing /assign @mboersma in a comment when ready.

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

The pull request process is described here

Details 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

@tariq1890
Copy link
Contributor

@sylr Thanks for this PR. Since goimports has changed significantly in how it performs it's linting, we would need to discuss with the rest of the aks-engine members if we want to go forward with this approach. We'll leave this PR open for the time being.

@mboersma
Copy link
Member

mboersma commented Dec 31, 2018

@sylr thanks so much for taking the initiative to fix this! I think we've worked around it in #205 by updating golangci-lint instead, so I'm going to close this PR.

@codecov
Copy link

codecov bot commented Jan 8, 2019

Codecov Report

Merging #204 into master will decrease coverage by 0.02%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #204      +/-   ##
==========================================
- Coverage   53.21%   53.19%   -0.03%     
==========================================
  Files          95       95              
  Lines       14230    14230              
==========================================
- Hits         7573     7569       -4     
- Misses       5991     5995       +4     
  Partials      666      666

@codecov
Copy link

codecov bot commented Jan 8, 2019

Codecov Report

Merging #204 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #204   +/-   ##
=======================================
  Coverage   53.25%   53.25%           
=======================================
  Files          95       95           
  Lines       14246    14246           
=======================================
  Hits         7586     7586           
  Misses       5995     5995           
  Partials      665      665

Signed-off-by: Sylvain Rabot <s.rabot@lectra.com>
@tariq1890
Copy link
Contributor

@sylr Can you rebase this ?

@sylr
Copy link
Contributor Author

sylr commented Jan 9, 2019

Ok

@tariq1890 tariq1890 changed the title Fix goimports checks fix: address goimports checks Jan 10, 2019
Copy link
Contributor

@tariq1890 tariq1890 left a comment

Choose a reason for hiding this comment

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

lgtm. Pending review of @jackfrancis and @mboersma

@mboersma mboersma merged commit b615a2e into Azure:master Jan 11, 2019
@sylr sylr deleted the goimports branch January 11, 2019 07:16
jackfrancis pushed a commit that referenced this pull request Jan 11, 2019
Signed-off-by: Sylvain Rabot <s.rabot@lectra.com>
juhacket pushed a commit to juhacket/aks-engine that referenced this pull request Mar 14, 2019
Signed-off-by: Sylvain Rabot <s.rabot@lectra.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants