Skip to content

Simplify travis setup a bit. #11056

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

Merged
merged 2 commits into from
May 10, 2018
Merged

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Apr 16, 2018

  • Splitting test_script.sh into its own file is a bit overkill (but then
    we can't use set -x as that applies to the whole script, using a
    subshell is a bit overkill).
  • pytest directly reads arguments from $PYTEST_ADDOPTS, use that.
  • We were setting $PYTHON_ARGS but not actually using it (that failed
    the legend docstring test), instead put a comment saying to set
    $PYTHONOPTIMIZE (which exists for that purpose).
  • ci/travis/setup.cfg can just as well be replaced by setting
    $MPLLOCALFREETYPE.
  • Yaml entries that are a single string don't need to get an extra
    indent.
  • matplotlibDeployKey.enc is unused since we switched to using circleci
    for doc builds.
  • ci/travis/silence was the only one left in its directory, so we may as
    well move it up once directory.

PR Summary

PR Checklist

  • Has Pytest style unit tests
  • Code is PEP 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

- Splitting test_script.sh into its own file is a bit overkill (but then
  we can't use `set -x` as that applies to the whole script, using a
  subshell is a bit overkill).
- pytest directly reads arguments from $PYTEST_ADDOPTS, use that.
- We were setting $PYTHON_ARGS but not actually using it (that failed
  the legend docstring test), instead put a comment saying to set
  $PYTHONOPTIMIZE (which exists for that purpose).
- ci/travis/setup.cfg can just as well be replaced by setting
  $MPLLOCALFREETYPE.
- Yaml entries that are a single string don't need to get an extra
  indent.
- matplotlibDeployKey.enc is unused since we switched to using circleci
  for doc builds.
- ci/travis/silence was the only one left in its directory, so we may as
  well move it up once directory.
.travis.yml Outdated
fi
before_install: |
# test with non-ascii in path
mkdir /tmp/λ
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand what this is doing, but why move this out of the if check?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There used to be an error when a directory in the PATH contained non-ascii characters on Linux (something with subprocess/Py2), so to test the fix we just added such a directory to the PATH. This patch just makes the directory also present in the OSX test, because why not?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe the comment could be expanded why we are doing this test and what we do/do not if it fails.
This is code one will only see once in a while and it‘s good to be clear about what it does.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually I pushed a commit that removes this (originally introduced in #7804 for #7715). It's a Py2-only thing which is just pointless in a Py3 world. Milestoning accordingly.

@anntzer anntzer added this to the v3.0 milestone May 10, 2018
@dopplershift dopplershift merged commit 46f8cad into matplotlib:master May 10, 2018
@anntzer anntzer deleted the simplertravis branch May 10, 2018 20:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants