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.

Sign up for GitHub

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

Jump to bottom

Upgrade to Go 1.7 #121

Merged
merged 1 commit into from
Sep 20, 2016
Merged

Upgrade to Go 1.7 #121

merged 1 commit into from
Sep 20, 2016

Conversation

zimmski
Copy link
Member

@zimmski zimmski commented Sep 8, 2016

No description provided.

@zimmski zimmski changed the title Upgrade to go 1.7 Upgrade to Go 1.7 Sep 8, 2016
@zimmski zimmski force-pushed the upgrade-to-go-1.7 branch 2 times, most recently from a807181 to 39785c8 Compare September 8, 2016 10:15
@zimmski zimmski changed the title Upgrade to Go 1.7 WIP Upgrade to Go 1.7 Sep 8, 2016
@zimmski zimmski changed the title WIP Upgrade to Go 1.7 Upgrade to Go 1.7 Sep 9, 2016
@zimmski
Copy link
Member Author

zimmski commented Sep 9, 2016

This PR is ready, I will integrate -msan if i find a solution to #123

sbinet
sbinet reviewed Sep 9, 2016
View reviewed changes
.travis.yml
@@ -1,6 +1,6 @@
language: go
go:
- 1.5
- 1.7
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't this be 1.7.1 ?

Copy link
Member Author

Choose a reason for hiding this comment

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

You mean be explicit about the patch version? ("1.7" uses the latest 1.7 release)

Copy link
Member Author

Choose a reason for hiding this comment

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

OK it should use the latest patch release but it does not https://travis-ci.org/go-clang/gen/builds/158708830

Copy link
Member Author

@zimmski zimmski Sep 9, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

done. I also added #124 to these PRs because I needed to change the CC env anyway.

@zimmski zimmski force-pushed the upgrade-to-go-1.7 branch 4 times, most recently from 86363f9 to f262c9b Compare September 9, 2016 12:34
@zimmski
Copy link
Member Author

zimmski commented Sep 9, 2016

Finally, the "env" TravisCI configuration was driving me crazy.

@zimmski
Copy link
Member Author

zimmski commented Sep 13, 2016

I still have not finished the address sanitizer issue but this PR is rebased and ready to go. Here is the corresponding bootstrap PR go-clang/bootstrap#11

@zimmski zimmski mentioned this pull request Sep 14, 2016
Open
@zimmski zimmski force-pushed the upgrade-to-go-1.7 branch 2 times, most recently from 84174fc to fb0bbbc Compare September 19, 2016 11:47
marxriedl
marxriedl reviewed Sep 19, 2016
View reviewed changes
Copy link
Contributor

@marxriedl marxriedl left a comment

Choose a reason for hiding this comment

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

LGTM in principle, apart from the full/verbose stuff

Makefile
CGO_LDFLAGS="-L`llvm-config --libdir`" go test -timeout 60s -race ./...
CGO_LDFLAGS="-L`llvm-config --libdir`" go test -timeout 60s ./...
test-full:
$(ROOT_DIR)/scripts/test-full.sh
test-verbose:
Copy link
Contributor

Choose a reason for hiding this comment

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

same as in bootstrap, why do we have both, test-full, test-verbose?

Copy link
Member Author

Choose a reason for hiding this comment

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

Let's discuss that in the other PR go-clang/bootstrap#11 (comment)

@zimmski
Upgrade to Go 1.7
76de4c0 
- Upgrades to Go 1.7
- Introduces test-throughout to test with the race detector and (new)
  with the address sanitizer if it is supported by the environment
- Cleanup of some scripts

Fixes #113 and #124
@zimmski zimmski merged commit 1a6b51e into master Sep 20, 2016
@zimmski zimmski deleted the upgrade-to-go-1.7 branch September 20, 2016 09:37
@zimmski zimmski restored the upgrade-to-go-1.7 branch September 20, 2016 09:59
@zimmski zimmski mentioned this pull request Sep 20, 2016
Closed
@zimmski zimmski deleted the upgrade-to-go-1.7 branch September 26, 2016 10:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marxriedl marxriedl marxriedl left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@zimmski @marxriedl @sbinet