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

go: gimme to installs go v1.12.x #1966

Closed
wants to merge 1 commit into from

Conversation

hunchback
Copy link
Collaborator

upgrade go to v1.12.x which supports go.mod dependencies

@hunchback hunchback force-pushed the go-v1.12.x branch 7 times, most recently from 62a0139 to 0e78a73 Compare September 5, 2019 13:28
@@ -14,7 +14,7 @@ curl -sL -o ~/bin/gimme https://raw.githubusercontent.com/travis-ci/gimme/master
chmod +x ~/bin/gimme

# before changing this be sure that it will not break the RHEL packaging
eval "$(gimme 1.10.3)"
eval "$(gimme 1.12.7)"
Copy link
Member

Choose a reason for hiding this comment

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

please update to 1.13

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok

packetinjector/server.go Show resolved Hide resolved
@@ -9,7 +9,7 @@
- name: run gimme in bash profile
lineinfile:
path: /home/vagrant/.bash_profile
line: 'eval "$(gimme 1.10.8)"'
line: 'eval "$(gimme 1.12.7)"'
Copy link
Member

Choose a reason for hiding this comment

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

1.13

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok

| grep -v '^tests') 2>&1 \
| grep -v '^$(SKYDIVE_GITHUB)/tests' \
| grep -v '^$(SKYDIVE_GITHUB)/contrib/collectd' \
) 2>&1 \
Copy link
Member

Choose a reason for hiding this comment

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

what about '^tests' ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

'^$(SKYDIVE_GITHUB)/tests' as I don't remove the prefix anymore

@test -z "$$($(GOVENDOR) fmt +local)" || \
(echo "+ please format Go code with 'gofmt -s'" && /bin/false)
$(GOVENDOR) fmt +local || \
(echo "+ please format Go code with '$(GOFMT)'" && /bin/false)
Copy link
Member

Choose a reason for hiding this comment

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

gofmt -s only we would like the output

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this is just the message to the user; and user should overwrite source (rather than output to stdout):

[vagrant@dev skydive]$ gofmt --help
usage: gofmt [flags] [path ...]
  -cpuprofile string
        write cpu profile to this file
  -d    display diffs instead of rewriting files
  -e    report all errors (not just the first 10 on different lines)
  -l    list files whose formatting differs from gofmt's
  -r string
        rewrite rule (e.g., 'a[b:len(a)] -> a[b:]')
  -s    simplify code
  -w    write result to (source) file instead of stdout
``

Makefile Outdated
@@ -32,9 +32,12 @@ $(call VENDOR_RUN,${PROTOC_GEN_GOFAST_GITHUB})
$(call VENDOR_RUN,${PROTOC_GEN_GO_GITHUB}) protoc -Ivendor -I. --plugin=${BUILD_TOOLS}/protoc-gen-gogofaster --gogofaster_out $$GOPATH/src $1
endef

export GO111MODULE=off
Copy link
Member

Choose a reason for hiding this comment

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

why turning of on 1.11 only ?
should be always auto

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

will remove leaving default. in 1.11 its' auto; it will change to on in future see: golang/go#31857

@hunchback
Copy link
Collaborator Author

replaced by: #1984

@hunchback hunchback closed this Sep 11, 2019
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.

2 participants