Travis CI migration and packaging improvements #2990

webknjaz · 2018-05-10T23:02:34Z

What do these changes do?

port Travis CI config changes from multidict
CI config polishing
CI links update
port dist setup changes
other packaging and CI related stuff
all changes are put into atomic commits making it possible to rebase-merge that as is

Are there changes in behavior for the user?

‒

Related issue number

‒

Checklist

I think the code is well written
Unit tests for the changes exist
Documentation reflects the changes
If you provide code modification, please add yourself to CONTRIBUTORS.txt
- The format is <Name> <Surname>.
- Please keep alphabetical order, the file is sorted by names.
Add a new news fragment into the CHANGES folder
- name it <issue_id>.<type> for example (588.bugfix)
- if you don't have an issue_id change it to the pr id after creating the pr
- ensure type is one of the following:
  - .feature: Signifying a new feature.
  - .bugfix: Signifying a bug fix.
  - .doc: Signifying a documentation improvement.
  - .removal: Signifying a deprecation or removal of public API.
  - .misc: A ticket has been closed, but it is not of interest to users.
- Make sure to use full sentences with correct case and punctuation, for example: "Fix issue with non-ascii contents in doctest text files."

asvetlov · 2018-05-11T05:31:44Z

setup.py

+        'CI: Shippable': 'https://app.shippable.com/github/{}'.format(repo_slug),
+        'CI: Travis': 'https://travis-ci.com/{}'.format(repo_slug),
+        'Coverage: codecov': 'https://codecov.io/github/{}'.format(repo_slug),
+        'Docs: RTD': 'https://{}.readthedocs.io'.format(name),


It is http://docs.aiohttp.org

asvetlov · 2018-05-11T05:34:02Z

setup.py


+name = 'aiohttp'


I don't like name substitution.
A plain constant is more obvious than string-formatted, isn't it?

I don't want to create code diversity, which often leads to forgetting to update some of places. Also, I use this approach in lots other distributions including multidict, which proved to be useful.

I've approved it for multidict but now I'm inclining to revert bare strings back.
It is written once but read many times.
Every time when I see the line I should remember what name is.
You've convinced me that we need overcomplicated travis config. Perhaps it is true, I'm not Travis expert.
But for setup.py I pretty sure that the simplest approach is the best.

I'd agree if we were using declarative setup.cfg, but this is code here.

By the way I have an 80-line shim snippet for supporting newer setuptools, which would allow us to move a vast of meta/options to setup.cfg

Here's how it would've looked in ansible if we accepted the patch: https://github.com/ansible/ansible/pull/38413/files#diff-380c6a8ebbbce17d55d50ef17d3cf906R1

+1 to move all to setup.cfg

@asvetlov
One good example I know:
https://github.com/malinoff/structures/blob/master/setup.cfg
https://github.com/malinoff/structures/blob/master/setup.py

I know how to do it, it's just not going to be in the same PR.

Also, there's certain complications related to this change. So let's not rush.

webknjaz · 2018-05-11T09:04:09Z

setup.py


+name = 'aiohttp'
+appveyor_slug = 'asvetlov/{}'.format(name)  # FIXME: move under aio-libs/* slug


@asvetlov how about finally fixing AppVeyor URL?

I don't remember how to setup it.
Could you do it yourself?

This requires owner privileges AFAIR, so I can't. It's a bit tricky, but the bottom line is that you create a team there mapped to the team in GitHub and give it some access there. Also you need to somehow create an org link there.

Here you go: https://www.appveyor.com/docs/team-setup/#setting-up-appveyor-account-for-github-organization
Create a separate account called aio-libs and then add your personal account as its administrator. You can also create team mapped to some team in GitHub, allowing its members to cancel/rerun builds.

webknjaz · 2018-05-14T07:58:47Z

@asvetlov please remove this deprecated integration continuous-integration/travis-ci/push Pending — The Travis CI build is in progress Required Details and make new Travis CI - Pull Request In progress — Build Started required instead.

asvetlov · 2018-05-14T08:20:16Z

@webknjaz I've added you to admin team.
I hope by this we can be more effective: instead asking me to press some buttons in github UI you can just do it yourself (but please discuss changes first :)

asvetlov · 2018-05-14T08:21:55Z

Anyway updated travis integration by your request

webknjaz · 2018-05-14T08:26:33Z

Sure, thanks :)

webknjaz · 2018-05-14T08:28:09Z

I'm still seeing it @ https://github.com/aio-libs/aiohttp/settings/installations.
Will remove it myself.

webknjaz · 2018-05-14T17:57:02Z

@asvetlov can we accept it now?

asvetlov

Almost good but I have a couple questions to discuss before merging

asvetlov · 2018-05-15T10:54:48Z

.appveyor.yml

@@ -14,10 +14,10 @@ install:
 build: false

 test_script:
-  - "tools/build.cmd %PYTHON%\\python.exe setup.py test"
+  - "tools/build.cmd %PYTHON%\\python.exe -m pytest tests"


Why -m? Technically setup.py is not an importable module but just a script.
Did I miss something?

Running setup.py test is considered bad practice and it's broken in this CI for some reason. Thus we have to run pytest as it's designed to work.

asvetlov · 2018-05-15T10:55:04Z

.appveyor.yml


 after_test:
-  - "tools/build.cmd %PYTHON%\\python.exe setup.py sdist bdist_wheel"
+  - "tools/build.cmd %PYTHON%\\python.exe -m setup sdist bdist_wheel"


it just looks cleaner, but doesn't really change things. you can run any python script/module this way. it's not only for packages.

I know I can but it looks confusing. At least for me.

It might look confusing for someone who is not used to it. It works though, I'm using it all the time now.

.travis.yml

lock · 2019-10-28T05:04:00Z

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a [new issue] for related bugs.
If you feel like there's important points made in this discussion, please include those exceprts into that [new issue].
[new issue]: https://github.com/aio-libs/aiohttp/issues/new

webknjaz requested a review from asvetlov May 10, 2018 23:02

asvetlov reviewed May 11, 2018

View reviewed changes

webknjaz commented May 11, 2018

View reviewed changes

webknjaz force-pushed the feature/travis-ci-migration-and-packaging branch 4 times, most recently from 5710686 to 2c9c521 Compare May 14, 2018 06:58

webknjaz added 11 commits May 14, 2018 09:12

Opt-in to use edge Travis CI config parser

ab017b3

Drop outdated comment from Travis CI config

4dd1cb7

Turn env vars in Travis CI config to key-value

a2f21b0

Move towncrier linter into its own job

dfa709c

Upgrade refs across repo to point to travis-ci.com

d242c61

Make run_docker script saver to maintain

1fb2194

Announce support for Python 3.7

b0223a0

Use pytest-runner in dist setup

9f6f891

Don't install mypy under PyPy

8396818

Don't install uvloop under 3.7

b705aaa

Run pytest directly in AppVeyor CI

9396516

webknjaz force-pushed the feature/travis-ci-migration-and-packaging branch from 2c9c521 to 9396516 Compare May 14, 2018 07:12

asvetlov mentioned this pull request May 14, 2018

Change appveyor account #2998

Closed

asvetlov reviewed May 15, 2018

View reviewed changes

asvetlov added 2 commits May 15, 2018 18:32

Polish job names

952c254

Use idiomatic python setup.py sdist

a045cff

asvetlov approved these changes May 17, 2018

View reviewed changes

asvetlov merged commit 1d3afb2 into master May 17, 2018

asvetlov deleted the feature/travis-ci-migration-and-packaging branch May 17, 2018 19:24

lock bot added the outdated label Oct 28, 2019

lock bot locked as resolved and limited conversation to collaborators Oct 28, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Travis CI migration and packaging improvements #2990

Travis CI migration and packaging improvements #2990

webknjaz commented May 10, 2018

asvetlov May 11, 2018

webknjaz May 11, 2018

asvetlov May 11, 2018

webknjaz May 11, 2018

asvetlov May 11, 2018

webknjaz May 11, 2018

webknjaz May 11, 2018

webknjaz May 14, 2018

kxepal May 15, 2018

kxepal May 15, 2018 •

edited

Loading

webknjaz May 15, 2018

webknjaz May 15, 2018

webknjaz May 11, 2018

asvetlov May 11, 2018

webknjaz May 11, 2018

webknjaz May 11, 2018

webknjaz commented May 14, 2018

asvetlov commented May 14, 2018

asvetlov commented May 14, 2018

webknjaz commented May 14, 2018

webknjaz commented May 14, 2018

webknjaz commented May 14, 2018

asvetlov left a comment

asvetlov May 15, 2018

webknjaz May 15, 2018

asvetlov May 15, 2018

asvetlov May 15, 2018

webknjaz May 15, 2018

asvetlov May 15, 2018

webknjaz May 15, 2018

lock bot commented Oct 28, 2019


		name = 'aiohttp'
		appveyor_slug = 'asvetlov/{}'.format(name) # FIXME: move under aio-libs/* slug

Travis CI migration and packaging improvements #2990

Travis CI migration and packaging improvements #2990

Conversation

webknjaz commented May 10, 2018

What do these changes do?

Are there changes in behavior for the user?

Related issue number

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kxepal May 15, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

webknjaz commented May 14, 2018

asvetlov commented May 14, 2018

asvetlov commented May 14, 2018

webknjaz commented May 14, 2018

webknjaz commented May 14, 2018

webknjaz commented May 14, 2018

asvetlov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lock bot commented Oct 28, 2019

kxepal May 15, 2018 •

edited

Loading