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

Travis tweaks #30761

Closed
wants to merge 5 commits into from
Closed

Travis tweaks #30761

wants to merge 5 commits into from

Conversation

jbytheway
Copy link
Contributor

Summary

SUMMARY: None

Purpose of change

After #30707 we lost some Travis builds I felt to be important. Rearrange things a bot more carefully.

Describe the solution

  • Re-enable gcc 5.3, gcc 8, and clang 3.8 builds.
  • Remove the clang 6 CMake build, and make the gcc 8 build be CMake instead, to test CMake support.
  • Disable the Linux Tiles gcc 8 build for PRs, and make the clang 3.8 build test Linux Tiles instead.
  • Set the gcc 5.3 build to have localization disabled (not testing this setting has caused issues a few times lately because devs don't tend to test it themselves).
  • Document in .travis.yml the things we're trying to cover with the builds we have.

Overall, that's one build more than before this PR; two fewer than before #30707.

Describe alternatives you've considered

Disabling the 'default' Linux Tiles build may be controversial, but it was the best compromise I could find; all aspects of that build were being tested elsewhere and I don't think I've ever seen it be the only build to surface an issue.

jbytheway added 5 commits May 22, 2019 20:46
We still want to test the CMake build, but this was two clang 6 builds;
move the CMake test to another build so we can remove the clang-6 one.
Use the clang-3.8 build as a Linux tiles build for PRs instead.
Use the gcc-5.3 build to test building with LOCALIZE=0.
@kevingranade
Copy link
Member

ahhhh I thought you weren't interested in doing this.
Well there's a competing PR at #30768

@ZhilkinSerg ZhilkinSerg added the Code: Build Issues regarding different builds and build environments label May 23, 2019
@jbytheway
Copy link
Contributor Author

Well, at a glance I think I prefer yours, but I encourage you to add some documentation in the .travis.yml file explaining why we've chosen what we have, similar to what I did on this PR.

@kevingranade
Copy link
Member

Yea I'm just going to steal yours 👍

kevingranade added a commit that referenced this pull request May 23, 2019
@ZhilkinSerg
Copy link
Contributor

So, I guess we can close this in favor of Kevins PR or will you repurpose it for something else later?

@kevingranade
Copy link
Member

I THINK now that mine has the strategy laid out, it has a superset of the functionality of this one. @jbytheway let us know if there's anything else you want to save from this.

ZhilkinSerg pushed a commit that referenced this pull request May 24, 2019
* Adjust coverage of Travis builds


Attempt to get in all the configurations we really care about in PR tests.
Max and Min versions of supported compilers.
A mix of Curses and Tiles/Sound builds.
A mix of Make and CMake builds.
Localization disabled.
The most popular (Tiles/Sound) Mac and Windows builds.
Address sanitization enabled on latest GCC and Clang.
Clang-tidy build.

Finally, round out all the remaining supported compiler versions only on master builds, along with the somewhat obscure Mingw-w64 Terminal build.

* Fix clang 8 install

* Outline travis build target strategy

Taken from #30761

* Add a new build stage
@jbytheway jbytheway closed this May 24, 2019
@jbytheway jbytheway deleted the travis_tweaks branch May 24, 2019 07:31
@jbytheway
Copy link
Contributor Author

I agree; closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code: Build Issues regarding different builds and build environments
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants