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

Drop support for Python 2.7 with next minor release #9

Merged

Conversation

csadorf
Copy link
Contributor

@csadorf csadorf commented Nov 15, 2019

  • Used AEP template from AEP 0
  • Status is submitted
  • Added type & status labels to PR
  • Added AEP to README.md
  • Provided github handles for authors

@csadorf
Copy link
Contributor Author

csadorf commented Nov 15, 2019

This AEP is to be interpreted as an alternative to this AEP.

@csadorf csadorf changed the title Drop python2 support with minor release Drop support for Python 2.7 with next minor release Nov 15, 2019
@ltalirz
Copy link
Member

ltalirz commented Nov 15, 2019

Hehe, should I now request changes on this? ;-)

csadorf and others added 3 commits November 15, 2019 18:14
Co-Authored-By: Leopold Talirz <leopold.talirz@gmail.com>
Co-Authored-By: Leopold Talirz <leopold.talirz@gmail.com>
Co-Authored-By: Leopold Talirz <leopold.talirz@gmail.com>
@csadorf
Copy link
Contributor Author

csadorf commented Nov 15, 2019

Hehe, should I now request changes on this? ;-)

I actually realized that it wouldn't be too much work to write the AEP for both alternatives and I found it a little easier to argue for this variant TBH. So I think we should have a quick discussion on which variant is generally preferred and then work out the details on that.

@csadorf csadorf added the type/process Process AEP label Nov 18, 2019
@csadorf csadorf force-pushed the drop-python2-support-with-minor-release branch from 818bccd to 95b9789 Compare November 19, 2019 09:16
For improved readability.
@csadorf csadorf force-pushed the drop-python2-support-with-minor-release branch from 2fb15a7 to 9d21fc9 Compare November 19, 2019 09:18
@ltalirz ltalirz self-requested a review November 19, 2019 10:07
@sphuber
Copy link
Contributor

sphuber commented Nov 19, 2019

Just for my understanding of the protocol: what determines the number of the AEP? Why 300? When it will be finally approved, just before merging will it take the next available subsequent number, i.e. 001? Or are they not supposed to be sequential?

@kjappelbaum
Copy link
Collaborator

I feel it should be consistent with https://www.python.org/dev/peps/pep-0001/#pep-editor-responsibilities-workflow. Though I think sequential would make most sense.

Assign a PEP number (almost always just the next available number, but sometimes it's a special/joke number, like 666 or 3141). (Clarification: For Python 3, numbers in the 3000s were used for Py3k-specific proposals. But now that all new features go into Python 3 only, the process is back to using numbers in the 100s again. Remember that numbers below 100 are meta-PEPs.

@ltalirz
Copy link
Member

ltalirz commented Nov 19, 2019

just before merging will it take the next available subsequent number, i.e. 001

yes, that was the idea + the status will be changed to "active".

If changed the branch protection rules such that further pushes to the branch don't dismiss an approval.
I think that's the practical solution - let me know if you would prefer another way.

@sphuber
Copy link
Contributor

sphuber commented Nov 19, 2019

So @csadorf did you chose 300 to keep 0-100 for meta and to indicate that this marks the start of python 3 only? (Since we probably won't be doing PEP's for py2 branches)

@csadorf
Copy link
Contributor Author

csadorf commented Nov 19, 2019

So @csadorf did you chose 300 to keep 0-100 for meta and to indicate that this marks the start of python 3 only? (Since we probably won't be doing PEP's for py2 branches)

I just chose it, because it seemed to fit and thought I got approval from @ltalirz , I guess that was a misunderstanding.

@csadorf
Copy link
Contributor Author

csadorf commented Nov 19, 2019

I think we should keep the number until next Tuesday for discussion purposes and then we can reassign one if we feel strongly about it before merging.

@csadorf
Copy link
Contributor Author

csadorf commented Nov 25, 2019

So far there have been no comments on the mailing list. Anything else that would need to be addressed prior to accepting this tomorrow?

@sphuber
Copy link
Contributor

sphuber commented Nov 25, 2019

I guess just to decide the numbering, but other than that I think this is good to go. I already prepared a PR to actually drop the support and clean up all the code. Dominik already gave it a pass, but it would be good to have others give a look as well, as well as try and go through the code base itself to see if I missed something. Then I want to release v1.0.1 with the current changes in develop and then merge the PR in develop, which will then become v1.1. I am also in far stage of moving CI to GitHub and with that done, we are ready for the coding week

@csadorf
Copy link
Contributor Author

csadorf commented Nov 25, 2019

I guess just to decide the numbering, but other than that I think this is good to go.

@ltalirz Since you are championing this AEP, are you going to take charge of that?

@ltalirz
Copy link
Member

ltalirz commented Nov 25, 2019

Right, then as the "AEP editor" I am assigning number 001
@csadorf could you please update the branch in your fork?

@sphuber @greschd We would need also one more approval to merge

@greschd
Copy link
Member

greschd commented Nov 25, 2019

I don't think my approval counts here 😄

@sphuber sphuber self-requested a review November 25, 2019 21:54
Copy link
Contributor

@sphuber sphuber left a comment

Choose a reason for hiding this comment

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

Alle trossen los!

@csadorf
Copy link
Contributor Author

csadorf commented Nov 26, 2019

@ltalirz Should be ready for merge.

@ltalirz ltalirz merged commit 26b8f50 into aiidateam:master Nov 26, 2019
@ltalirz
Copy link
Member

ltalirz commented Nov 26, 2019

Aaand we have our first AEP merged. Thanks a bunch @csadorf !

@csadorf csadorf deleted the drop-python2-support-with-minor-release branch November 26, 2019 11:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants