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.

Sign up for GitHub

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

Jump to bottom

Prioritise 64-bit downloads over 32-bit #2311

Merged
merged 5 commits into from
Feb 21, 2024
Merged

Prioritise 64-bit downloads over 32-bit #2311

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0403ca1
Prioritise 64-bit downloads over 32-bit
hugovk Sep 16, 2023
6509e25
Put Windows embeddable packages at the end
hugovk Sep 17, 2023
dadd924
Define explict preferred order for Windows files
hugovk Sep 29, 2023
85864ce
Merge branch 'main' into prioritise_64bit_over_32bit
hugovk Jan 21, 2024
29a2857
Fix conflict resolution
hugovk Jan 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions downloads/templatetags/download_tags.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,37 @@ def has_sigstore_materials(files):
f.sigstore_bundle_file or f.sigstore_cert_file or f.sigstore_signature_file
for f in files
)


@register.filter
def sort_windows(files):
if not files:
return files

# Put Windows files in preferred order
files = list(files)
windows_files = []
other_files = []
for preferred in (
'Windows installer (64-bit)',
Copy link
Member

Choose a reason for hiding this comment

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

Are these strings generated by code elsewhere in the codebase or manually input by Release Managers?

If it is manual input, this feels brittle and likely to cause confusion.

Copy link
Member

Choose a reason for hiding this comment

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

They appear to be strings stored on the ReleaseFile model via the NameSlugModel mixin. I'm pretty iffy on handling this this way honestly.

Copy link
Member

Choose a reason for hiding this comment

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

@Yh1gs for RM input

Copy link
Member

Choose a reason for hiding this comment

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

Yes, they're freeform strings, set by the script adding the ReleaseFile objects: https://github.com/python/release-tools/blob/master/add-to-pydotorg.py#L90.

Copy link
Member Author

Choose a reason for hiding this comment

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

Before checking for the exact strings, I had something that checked for substrings like "(64-bit)", "(32-bit)" and "Windows embeddable package", see the old version at: dadd924 (#2311)

Would you prefer something like that? Or some other approach?

Copy link
Member

Choose a reason for hiding this comment

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

IMHO having the strings is fine, if comments were added to reference this file from the other and vice-versa

'Windows installer (32-bit)',
'Windows installer (ARM64)',
'Windows help file',
'Windows embeddable package (64-bit)',
'Windows embeddable package (32-bit)',
'Windows embeddable package (ARM64)',
):
for file in files:
if file.name == preferred:
windows_files.append(file)
files.remove(file)
break

# Then append any remaining Windows files
for file in files:
if file.name.startswith('Windows'):
windows_files.append(file)
else:
other_files.append(file)

return other_files + windows_files
5 changes: 3 additions & 2 deletions templates/downloads/os_list.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{% extends "downloads/base.html" %}
{% load boxes %}
{% load sitetree %}
{% load sort_windows from download_tags %}

{% block body_attributes %}class="python download"{% endblock %}

Expand Down Expand Up @@ -45,7 +46,7 @@ <h2>Stable Releases</h2>
{% endif %}
{% endif %}
<ul>
{% for f in r.files.all %}
{% for f in r.files.all|sort_windows %}
<li>Download <a href="{{ f.url }}">{{ f.name }}</a></li>
{% empty %}
<li>No files for this release.</li>
Expand All @@ -63,7 +64,7 @@ <h2>Pre-releases</h2>
<li>
<a href="{{ r.get_absolute_url }}">{{ r.name }} - {{ r.release_date|date }}</a>
<ul>
{% for f in r.files.all %}
{% for f in r.files.all|sort_windows %}
<li>Download <a href="{{ f.url }}">{{ f.name }}</a></li>
{% empty %}
<li>No files for this release.</li>
Expand Down
3 changes: 2 additions & 1 deletion templates/downloads/release_detail.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
{% load boxes %}
{% load sitetree %}
{% load has_sigstore_materials from download_tags %}
{% load sort_windows from download_tags %}

{% block body_attributes %}class="python downloads"{% endblock %}

Expand Down Expand Up @@ -56,7 +57,7 @@ <h1 class="page-title">Files</h1>
</tr>
</thead>
<tbody>
{% for f in release_files %}
{% for f in release_files|sort_windows %}
<tr>
<td><a href="{{ f.url }}">{{ f.name }}</a></td>
<td>{{ f.os.name }}</td>
Expand Down
Loading