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

Start using goimports #317

Merged
merged 7 commits into from Aug 24, 2018

Conversation

Projects
None yet
3 participants
@Waldz
Copy link
Member

commented Aug 23, 2018

No description provided.

@Waldz Waldz requested review from tadovas, interro, soffokl, zolia and mysteriumnetwork/core Aug 23, 2018

@tadovas
Copy link
Member

left a comment

Overall looks ok. Just some small things

bin/check Outdated
bin/check_fmt
bin/check_govet
bin/check_golint
bin/check_gofmt

This comment has been minimized.

Copy link
@tadovas

tadovas Aug 23, 2018

Member

goimports covers gofmt, therefore gofmt is not needed anymore

This comment has been minimized.

Copy link
@Waldz

Waldz Aug 23, 2018

Author Member

Removed gofmt

.travis.yml Outdated

- script: bin/check_fmt
name: "Formating check"
- script: bin/check_gofmt

This comment has been minimized.

Copy link
@tadovas

tadovas Aug 23, 2018

Member

Not needed if using goimports

This comment has been minimized.

Copy link
@Waldz

Waldz Aug 23, 2018

Author Member

Removed

"fmt"

This comment has been minimized.

Copy link
@tadovas

tadovas Aug 23, 2018

Member

The order of imports here looks strange :/ Standart ones - our custom - standart again?

This comment has been minimized.

Copy link
@Waldz

Waldz Aug 23, 2018

Author Member

Fixed. When I accidently add to many newlines, goimports dont reformat my "better style"

@Waldz Waldz changed the title HOTFIX Start using goimports Start using goimports Aug 23, 2018

@@ -60,5 +60,5 @@ if [ ${#MESSAGES_RECONFIGURE[@]} -gt 0 ]; then
exit 1
fi

print_success "All packages are clean"
print_success "All packages are are compliant to golint"

This comment has been minimized.

Copy link
@soffokl

soffokl Aug 23, 2018

Member

double "are"

This comment has been minimized.

Copy link
@Waldz

Waldz Aug 23, 2018

Author Member

Fixed

@Waldz Waldz force-pushed the Waldz:hotfix/install-goimports branch from b11dd1a to 1f665fa Aug 23, 2018

@tadovas
Copy link
Member

left a comment

Lgtm

@@ -25,9 +25,10 @@ import (
"time"

This comment has been minimized.

Copy link
@soffokl

soffokl Aug 23, 2018

Member

one more extra line

This comment has been minimized.

Copy link
@Waldz

Waldz Aug 24, 2018

Author Member

Fixed


source bin/helpers/output.sh

unformatted=`find . \

This comment has been minimized.

Copy link
@soffokl

soffokl Aug 23, 2018

Member

this can be done simpler way: goimports -e -l $(go list -f {{.Dir}} ./...)

This comment has been minimized.

Copy link
@Waldz

Waldz Aug 24, 2018

Author Member

Improved

@@ -23,6 +23,7 @@ import (
"testing"

This comment has been minimized.

Copy link
@soffokl

soffokl Aug 23, 2018

Member

extra empty line

This comment has been minimized.

Copy link
@Waldz

Waldz Aug 24, 2018

Author Member

Fixed

@@ -21,13 +21,14 @@ import (
"fmt"
"net/http"

This comment has been minimized.

Copy link
@soffokl

soffokl Aug 23, 2018

Member

extra empty line

This comment has been minimized.

Copy link
@Waldz

Waldz Aug 24, 2018

Author Member

Fixed

Waldz added some commits Aug 23, 2018

@Waldz Waldz force-pushed the Waldz:hotfix/install-goimports branch from 1f665fa to c578134 Aug 24, 2018

@soffokl
Copy link
Member

left a comment

Looks good to me. Thank you for the changes.

@tadovas
Copy link
Member

left a comment

reLGTMed

@Waldz Waldz merged commit 01b5b40 into mysteriumnetwork:master Aug 24, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@Waldz Waldz deleted the Waldz:hotfix/install-goimports branch Aug 24, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.