Skip to content

Add Travis CI support #3

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
Merged
cfournie merged 4 commits into from
Mar 2, 2017
Merged

Add Travis CI support #3

cfournie merged 4 commits into from
Mar 2, 2017

Conversation

@cfournie
Copy link
Contributor

@cfournie cfournie commented Mar 1, 2017 •
edited
Loading

This PR adds support for Travis CI including:

  • Python 2.7, 3.3, 3.4, 3.5, and 3.6
  • Pylint on all Python versions
  • Mypy Python 2-compatible type annotation checking for Python 3.x only

To accomplish this, a modern requirement specifier was used in setup.py'sextras_require which necessitated upgrading setuptools on Travis CI.

@cfournie cfournie force-pushed the travis branch 5 times, most recently from b95b814 to 74e3908 Compare March 2, 2017 14:23
@cfournie cfournie changed the title [WIP] Add Travis CI support Add Travis CI support Mar 2, 2017
JasonMWhite
JasonMWhite approved these changes Mar 2, 2017
View reviewed changes
Copy link

@JasonMWhite JasonMWhite left a comment

Choose a reason for hiding this comment

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

Awesome!

@honkfestival
Copy link

honkfestival commented Mar 2, 2017

What reasons do we have for keeping the Makefile around? I'm not opposed to make in general, but everyone using this repo is going to end up using it. If there's another way that's just as easy (i.e. if the Makefile isn't adding any value), I think we should remove it.

@cfournie
Copy link
Contributor Author

cfournie commented Mar 2, 2017 •
edited
Loading

The Makefile's uses right now include:

  • Setting up the dev environment
  • Executing tests
  • Linting

Specifying all of these activities in one place standardizes them for both local machines and CI. For that reason, we should script these activities somewhere. Our options include:

  • Something language-specific (e.g. Java's maven) of which Python has many options but no clear leaders;
  • Shell scripts (e.g. those found in a project's bin folder); or
  • make (which is really just a centralized location for shell scripts and dependency management)

The Makefile adds value in terms of standardizing and making development activities easier to perform. There are other tools that we could probably adopt to accomplish the same outcomes.

@honkfestival
Copy link

honkfestival commented Mar 2, 2017

The Makefile adds value in terms of standardizing and making development activities easier to perform.

Sorry, I should have been more clear: I was really asking about make vs other alternatives which might be easier to learn than make if you don't know either.

... of which Python has many options but no clear leaders

In that case, let's go with make.

@cfournie
Copy link
Contributor Author

cfournie commented Mar 2, 2017

In that case, let's go with make.

I'm going to give tox a try first, buildout is another option if we ever start spanning languages.

@honkfestival
Copy link

honkfestival commented Mar 2, 2017

Doesn't make operate at a higher level of abstraction (i.e. dependent tasks)? Using tox is great, but I suspect you'll want to call tox from make instead of replacing it.

@cfournie
Copy link
Contributor Author

cfournie commented Mar 2, 2017

I suspect you'll want to call tox from make instead of replacing it.

I tried my best to replace make using tox, but it's clumsier, slower, and harder to read by comparison.

@cfournie cfournie requested a review from erikwright March 2, 2017 20:35
erikwright
erikwright reviewed Mar 2, 2017
View reviewed changes
Makefile
@@ -1,4 +1,6 @@
python_files := find . -path '*/.*' -prune -o -name '*.py' -print0
Copy link
Contributor

@erikwright erikwright Mar 2, 2017

Choose a reason for hiding this comment

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

License boilerplate

erikwright
erikwright reviewed Mar 2, 2017
View reviewed changes
.travis.yml
@@ -0,0 +1,15 @@
language: python
Copy link
Contributor

@erikwright erikwright Mar 2, 2017

Choose a reason for hiding this comment

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

License boilerplate

erikwright
erikwright reviewed Mar 2, 2017
View reviewed changes
setup.py

try:
from setuptools import setup
import setuptools as setuplib
Copy link
Contributor

@erikwright erikwright Mar 2, 2017

Choose a reason for hiding this comment

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

Any reason for this to be conditional? Not the least because you are apparently depending on it now.

Copy link
Contributor Author

@cfournie cfournie Mar 2, 2017

Choose a reason for hiding this comment

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

This try/catch allows someone to install this locally if they don't have setuptools installed, e.g. using python setup.py install. That means that they can't get any of the extras (that's a setuptools feature) but they can at least install the package.

erikwright
erikwright reviewed Mar 2, 2017
View reviewed changes
setup.py
],
extras_require={
'dev': [
'autopep8',
Copy link
Contributor

@erikwright erikwright Mar 2, 2017

Choose a reason for hiding this comment

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

extra_requires is really neat.

@erikwright
Copy link
Contributor

erikwright commented Mar 2, 2017

LGTM

@cfournie cfournie merged commit 7c9a53b into master Mar 2, 2017
@cfournie cfournie deleted the travis branch March 2, 2017 21:17
zeedann added a commit that referenced this pull request Apr 12, 2017
@zeedann
# This is a combination of 4 commits.
ba7a74c 
# This is the 1st commit message:
added some tets

# This is the commit message #2:

show me numbers not types

# This is the commit message #3:

tests and giggles

# This is the commit message #4:

linted
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@erikwright erikwright erikwright left review comments

+2 more reviewers

@honkfestival honkfestival honkfestival approved these changes

@JasonMWhite JasonMWhite JasonMWhite approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@cfournie @honkfestival @erikwright @JasonMWhite