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

Add Active Python Releases to Downloads Page #1569

Merged

Conversation

crwilcox
Copy link
Contributor

#1302 raised that it would be nice to have information, similar to other languages, that clearly states which python versions are actively maintained.

image

Copy link
Member

@Mariatta Mariatta left a comment

Choose a reason for hiding this comment

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

The table looks good to me. Thank you! I think this will help the community greatly.

@Mariatta
Copy link
Member

@berkerpeksag will you be able to review and also deploy this to prod? Or if you can tell me how I can deploy to production, then I'll be able to help.

@ewdurbin
Copy link
Member

ewdurbin commented Apr 1, 2020

Thank you @crwilcox for the implementation and @Mariatta for the idea and review!

My only concern here is with populating the table staticly in the template will require someone to update the template as new releases of python come into existence.

The @python/python-release-managers would probably be best suited to maintain this information as they already interact with the Release objects when performing their duties.

I'm not positive if it's worth blocking this on these concerns, but adding a new Version model to replace the very rough versions field on Release would make this far more maintainable as part of a RM's role, and allow the list to be populated from the database rather than being hard coded in the template.

I'll leave some time for @python/python-release-managers to chime in with any thoughts, but otherwise this can be merged and the above ramblings converted into an issue :)

@crwilcox
Copy link
Contributor Author

crwilcox commented Apr 1, 2020

Thanks for taking a look @ewdurbin! I raised a similar concern on the issue. I think linking this to releases would be great. I wasn't quite sure the approach that would be most desired by release managers so I sort of stopped myself 😅

The coupling today of releases and updating PEPs/Dev Guide isn't super automated (as best I know anyway), but it all seems to work out. That said, I would be happy to take on getting a model setup for this, though as you said it might be easier to do that as a separate PR.

Copy link
Member

@berkerpeksag berkerpeksag left a comment

Choose a reason for hiding this comment

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

Thank you for the PR! In general, I agree with @ewdurbin that it would be better to find a more dynamic solution, but I think this is a good start for linking release schedule PEPs on www.python.org. If RMs will be happy about getting this in, it would be nice to update PEP 101 and include it as part of release process.

I left some comments about HTML and duplicate template tag.

@@ -37,6 +37,59 @@ <h1 class="call-to-action">Download the latest version of Python</h1>
{% endblock header_content %}

{% block content %}
<div class="row active-release-list-widget">
{% render_active_banner %}
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to move this inside <div class="row download-list-widget">? If not, we need to remove the duplicate render_active_banner call in line 95.

Copy link
Member

Choose a reason for hiding this comment

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

Actually, you can ignore my comment about moving it inside <div class="row download-list-widget">. It looks pretty great! :) We just need to remove the duplicate render_active_banner call in line 95.

templates/downloads/index.html Outdated Show resolved Hide resolved
templates/downloads/index.html Outdated Show resolved Hide resolved
@ned-deily ned-deily self-requested a review April 6, 2020 03:26
@ned-deily
Copy link
Member

Thanks for the PR. On behalf of the release managers, who maintain this information, I'd like to take a closer review of this over the next few days.

crwilcox and others added 3 commits April 13, 2020 08:56
Co-Authored-By: Berker Peksag <berker.peksag@gmail.com>
Co-Authored-By: Berker Peksag <berker.peksag@gmail.com>
@crwilcox
Copy link
Contributor Author

Thanks for the feedback @berkerpeksag. I think I have addressed all of it. @ned-deily have you had a chance to think about this? Are there other release folks worthing having take a look?

templates/downloads/index.html Outdated Show resolved Hide resolved
templates/downloads/index.html Outdated Show resolved Hide resolved
crwilcox and others added 2 commits April 20, 2020 08:58
Co-Authored-By: Hugo van Kemenade <hugovk@users.noreply.github.com>
Co-Authored-By: Hugo van Kemenade <hugovk@users.noreply.github.com>
Copy link
Member

@ned-deily ned-deily left a comment

Choose a reason for hiding this comment

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

Sorry about the review delay! I think it's a big improvement to the web site. Thanks for the PR. Obviously it's not ideal to have the release data hardwired into the templates but it will do for now. Longer term, I'd like to find a way to extract the release information we have from the database and make it available as, say, a json file on the website that could then be used by multiple consumers including this web page. But that's not a good reason to hold this up. I will add a note to PEP 101, the Python Release Process document to remind release managers to update this data on the website at appropriate points in our release cycle. Thanks again, everyone!

Copy link
Member

@berkerpeksag berkerpeksag left a comment

Choose a reason for hiding this comment

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

LGTM!

@berkerpeksag berkerpeksag merged commit 7e45c5d into python:master Apr 20, 2020
@ewdurbin
Copy link
Member

Thanks @crwilcox! This is now live.

@ned-deily
Copy link
Member

ned-deily commented Apr 20, 2020

You know how you look at something too long and you overlook the obvious?? I've opened #1577 to change 2.7 status from bugfix to end-of-life (and then remove in the future).

@berkerpeksag
Copy link
Member

Oops, I completely missed it when I looked at it before merging :)

@ned-deily
Copy link
Member

We all did! I guess it's still a bit hard to believe that 2.7 is gone :)

@crwilcox
Copy link
Contributor Author

Thanks to everyone for helping get this in.

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.

6 participants