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

Set up CI with Azure Pipelines #3827

Closed
wants to merge 81 commits into from
Closed

Set up CI with Azure Pipelines #3827

wants to merge 81 commits into from

Conversation

asvetlov
Copy link
Member

@asvetlov asvetlov commented Jun 8, 2019

No description provided.

@asvetlov asvetlov requested a review from webknjaz as a code owner June 8, 2019 00:42
@samuelcolvin
Copy link
Member

Great idea. Let me know if you want any help.

@asvetlov
Copy link
Member Author

asvetlov commented Jun 8, 2019

I'd like to get an initial working setup and iterate over in separate requests.
40 min per PR for appveyor is too much

@webknjaz
Copy link
Member

webknjaz commented Jun 8, 2019

Here's a very good curated template for Azure Pipelines: https://github.com/tox-dev/azure-pipelines-template

@webknjaz
Copy link
Member

webknjaz commented Jun 8, 2019

But yes, we can merge whatever works initially and iterate on that later.

setup.py Outdated Show resolved Hide resolved
@@ -1,5 +1,3 @@
"""HTTP client functional tests against aiohttp.web server"""
Copy link
Member

Choose a reason for hiding this comment

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

Please keep the module docstring.

Copy link
Member Author

Choose a reason for hiding this comment

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

Why? What is the point?

Copy link
Member

Choose a reason for hiding this comment

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

When I open a python module, regardless of whether it is a test suite or not it helps me to understand what I'm about to see next, get some context and decide whether I need to read further.

Copy link
Member Author

@asvetlov asvetlov Jun 14, 2019

Choose a reason for hiding this comment

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

I have a different position: docstrings are for public API only.
It should be just a comment otherwise.

@@ -1200,7 +1198,7 @@ def do_release():
app.router.add_post('/', handler)
client = await aiohttp_client(app)

with fname.open() as f:
Copy link
Member

Choose a reason for hiding this comment

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

It'd be nice to move this to a separate PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

The test fails on azure but is passed on appveyor

Copy link
Member

Choose a reason for hiding this comment

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

Okay, but I still think that it'd be nice to separate this and only keep the CI config here so that it'd be more atomic.

Copy link
Member Author

Choose a reason for hiding this comment

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

I cannot test it in a separate PR, this is the problem.

@webknjaz webknjaz closed this Jul 29, 2019
@webknjaz webknjaz reopened this Jul 29, 2019
@webknjaz
Copy link
Member

I think it may make sense to focus on GitHub workflows as they provide more resources.

@asvetlov
Copy link
Member Author

Like Windows and MacOS boxes?

@webknjaz
Copy link
Member

Yes

@asvetlov
Copy link
Member Author

"GitHub Actions is currently in limited public beta and is subject to change. We strongly recommend that you do not use this feature for high-value workflows and content during the beta period."

I want to use a tool, not fight with unstable yet features. aiohttp itself provides a lot of space for experimenting.

@webknjaz
Copy link
Member

I think they forgot to update that. Basically, anybody applying for access gets it instantly now and it's going to be generally available on Nov 13.
I've been playing with it for a while and I'm happy with the results.

@asvetlov
Copy link
Member Author

Done by separate PRs

@asvetlov asvetlov closed this Oct 14, 2019
@asvetlov asvetlov deleted the azure-pipelines branch October 14, 2019 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants