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

Simplify CI by deleting redundant production of source distribution #590

Merged
merged 2 commits into from Oct 15, 2017

Conversation

Projects
None yet
3 participants
@lucafavatella
Contributor

lucafavatella commented Oct 8, 2017

Please refer to commit message for details.

I did not test upload to PyPi.

Simplify CI by deleting redundant production of source distribution
The command `python setup.py sdist` produces the archive meant to be
uploaded to pypi.  The archive is currently being uploaded by any CI
build running on the `master` branch.

Any CI build on the `master` branch currently runs `python setup.py
sdist` twice:
* Once during the `install` phase;
* Then, *after having cleaned the working directory*, by the `deploy`
  phase too.  More details on this further below.

The only effect of the `python setup.py sdist` run during the
`install` phase appears to be that PRs are checked for being able to
produce a source distribution.  This appears not designed on purpose,
as anyway `python setup.py install` shall detect all relevant major
issues.

So `python setup.py sdist` in the `install` phase appears redundant,
therefore delete it.

----

The CI `deploy` phase already runs `python setup.py sdist` - see
[doc](https://docs.travis-ci.com/user/deployment/pypi/#Uploading-different-distributions)
and code
[1](https://github.com/travis-ci/dpl/blob/v1.8.43/lib/dpl/provider/pypi.rb#L101)
[2](https://github.com/travis-ci/dpl/blob/v1.8.43/lib/dpl/provider/pypi.rb#L20).

The CI cleans the working directory before running the `deploy` phase
hence making the early `python setup.py sdist` ineffective.  As the
`skip_cleanup` option is not specified, before executing the `deploy`
phase the CI [runs `git stash
--all`](https://docs.travis-ci.com/user/deployment/#Uploading-Files),
that cleans the working directory [including untracked files and
ignored files](https://www.git-scm.com/docs/git-stash#_options) - as
can be seen by inspecting the Travis CI log of recent builds on master
branch:
> Cleaning up git repository with `git stash --all`. If you need build
  artifacts for deployment, set `deploy.skip_cleanup: true`. See
  https://docs.travis-ci.com/user/deployment/#Uploading-Files.
@codecov-io

This comment has been minimized.

Show comment
Hide comment
@codecov-io

codecov-io Oct 8, 2017

Codecov Report

Merging #590 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #590   +/-   ##
=======================================
  Coverage   86.18%   86.18%           
=======================================
  Files         131      131           
  Lines        8217     8217           
=======================================
  Hits         7082     7082           
  Misses       1135     1135

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a3f7885...a6a880f. Read the comment docs.

codecov-io commented Oct 8, 2017

Codecov Report

Merging #590 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #590   +/-   ##
=======================================
  Coverage   86.18%   86.18%           
=======================================
  Files         131      131           
  Lines        8217     8217           
=======================================
  Hits         7082     7082           
  Misses       1135     1135

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a3f7885...a6a880f. Read the comment docs.

@kylepjohnson

This comment has been minimized.

Show comment
Hide comment
@kylepjohnson

kylepjohnson Oct 9, 2017

Member

Excellent explanation. I will read a few things about this myself, though I am sure you're correct.

Member

kylepjohnson commented Oct 9, 2017

Excellent explanation. I will read a few things about this myself, though I am sure you're correct.

@kylepjohnson

Looks good, I'll verify this on out next version/release.

@kylepjohnson kylepjohnson merged commit 76ddd93 into cltk:master Oct 15, 2017

3 checks passed

codecov/patch Coverage not affected when comparing a3f7885...a6a880f
Details
codecov/project 86.18% remains the same compared to a3f7885
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@lucafavatella lucafavatella deleted the lucafavatella:ci-sdist-dedup branch Oct 20, 2017

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