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

Sort globbed files for releases #52

Merged
merged 1 commit into from
Jul 20, 2022
Merged

Sort globbed files for releases #52

merged 1 commit into from
Jul 20, 2022

Conversation

beyarkay
Copy link
Contributor

@beyarkay beyarkay commented Jul 16, 2022

This change simply sorts the list of files after the glob has been resolved. This is useful because these files are often destined for a release, and having to scroll through hundreds of files which aren't sorted is painful.

New Features

None

Changes

  • Sorts files before they're uploaded so that the user sees a nice, sorted list of files on the releases page.

Bug Fixes

None


I'm happy to convert this into an option that can be specified in the yaml if you'd prefer?

This change simply sorts the list of files after the glob has been resolved. This is useful because these files are often destined for a release, and having to scroll through hundreds of files which aren't sorted is *painful*.

I'm happy to convert this into an option that can be specified in the yaml if you'd prefer?
@Paebbels Paebbels added Enhancement New feature or request Releaser Action 'releaser' labels Jul 16, 2022
@Paebbels Paebbels requested a review from umarcor July 16, 2022 11:57
@Paebbels
Copy link
Member

@umarcor I see no harm in this new behavior, so it doesn't need to be a parameter, right?

@beyarkay
Copy link
Contributor Author

beyarkay commented Jul 16, 2022

Yeah no harm at all, just wasn't sure how strict you were gonna be about the O(nlogn) time to sort the array 😄. By way of an extra example, here's a release I've got. Trying to find the calendar western-cape-stellenbosch.ics if you're a luddite and don't know about ctrl-f is a bit tricky.

But thanks for the GH action! it's solved so many issues I had previously.

@beyarkay
Copy link
Contributor Author

Is there anything preventing this from being merged?

Copy link
Member

@umarcor umarcor left a comment

Choose a reason for hiding this comment

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

LGTM!

@umarcor umarcor merged commit 31f02bb into pyTooling:main Jul 20, 2022
@beyarkay beyarkay deleted the patch-1 branch September 30, 2022 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request Releaser Action 'releaser'
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants