Skip to content
This repository has been archived by the owner on Apr 6, 2023. It is now read-only.

Fix issue 12 and extend CI tests to additional Python versions/OSes #13

Merged
merged 6 commits into from
Nov 25, 2020

Conversation

lbianchi-lbl
Copy link
Contributor

Fixes #12

Proposed changes:

  • Add an explicit Python version check to exclude incompatible versions from workaround
  • Extend CI tests so that they run on Python version/OS combination matrix

Legal Acknowledgement

By contributing to this software project, I agree to the following terms and conditions for my contribution:

  1. I agree my contributions are submitted under the license terms described in the LICENSE.txt file at the top level of this directory.
  2. I represent I am authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.

@lbianchi-lbl lbianchi-lbl added the bug Something isn't working label Nov 20, 2020
@lbianchi-lbl lbianchi-lbl self-assigned this Nov 20, 2020
build.py Outdated Show resolved Hide resolved
dangunter
dangunter previously approved these changes Nov 20, 2020
Copy link
Member

@dangunter dangunter left a comment

Choose a reason for hiding this comment

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

Looks good

andrewlee94
andrewlee94 previously approved these changes Nov 20, 2020
Copy link
Member

@andrewlee94 andrewlee94 left a comment

Choose a reason for hiding this comment

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

Rubber stamp.

@lbianchi-lbl lbianchi-lbl dismissed stale reviews from andrewlee94 and dangunter via a7fcb0c November 20, 2020 18:55
@lbianchi-lbl
Copy link
Contributor Author

lbianchi-lbl commented Nov 20, 2020

@dangunter not sure what's the status for Python 3.9 but we can try to see how it behaves if we remove the workaround once we start supporting 3.9.

In the meanwhile, what I'm getting from tornadoweb/tornado#2608 is that, for the time being, this should be dealt with on the side of the application: so our workaround is the recommended way to address this. In any case he issue looks quite active and we can use it as a reference to see if there's any progress.

I've added this information as a comment in the code, so if you're OK with the wording you can go ahead and re-approve it.

Copy link
Member

@andrewlee94 andrewlee94 left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@ksbeattie ksbeattie added the Priority:High High Priority Issue or PR label Nov 25, 2020
Copy link
Member

@ksbeattie ksbeattie left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working Priority:High High Priority Issue or PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

build.py errors for Python 3.6 on Windows
4 participants