-
Notifications
You must be signed in to change notification settings - Fork 601
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
Add Active Python Releases to Downloads Page #1569
Conversation
There was a problem hiding this 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.
@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. |
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 I'm not positive if it's worth blocking this on these concerns, but adding a new 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 :) |
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. |
There was a problem hiding this 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 %} |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
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. |
Co-Authored-By: Berker Peksag <berker.peksag@gmail.com>
Co-Authored-By: Berker Peksag <berker.peksag@gmail.com>
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? |
Co-Authored-By: Hugo van Kemenade <hugovk@users.noreply.github.com>
Co-Authored-By: Hugo van Kemenade <hugovk@users.noreply.github.com>
There was a problem hiding this 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!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Thanks @crwilcox! This is now live. |
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). |
Oops, I completely missed it when I looked at it before merging :) |
We all did! I guess it's still a bit hard to believe that 2.7 is gone :) |
Thanks to everyone for helping get this in. |
#1302 raised that it would be nice to have information, similar to other languages, that clearly states which python versions are actively maintained.