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

script/query: don't restrict ./script.sh get-latest to a single tag #350

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tleb
Copy link
Member

@tleb tleb commented Nov 15, 2024

While fixing #349 I noticed the oddity that was script.sh get-latest. On one side we have script.sh that has access to the full tag list but only returns one, and on the other we have the Python code that spawns multiple time script.sh if the tags it gets aren't up to its expectations.

Simplify that by making script.sh return the full sorted list of tags. Then we let Python code do its filtering based on database content. Code is more straight forward that way.


Merged 3d81fa4 quickly to fix the ugly user-facing bug, but this is refactoring so let's do a round of review.


Commit message:

./script.sh get-latest <offset> gets the full list of tags, filters it, sorts it then returns a single result. On the Python side, it gets the first one. If that works, it uses it, else it tries the second one, etc.

That is a weird implementation: modify get-latest to return all tags so that Python code only has to spawn a single subprocess.

Also, rename it from get-latest to get-latest-tags. This makes things more explicit (what latest?) and also explicits that more than one tag is required.

`./script.sh get-latest <offset>` gets the full list of tags, filters
it, sorts it then returns a single result. On the Python side, it gets
the first one. If that works, it uses it, else it tries the second one,
etc.

That is a weird implementation: modify get-latest to return all tags so
that Python code only has to spawn a single subprocess.

Also, rename it from `get-latest` to `get-latest-tags`. This makes
things more explicit (what latest?) and also explicits that more than
one tag is required.

Signed-off-by: Théo Lebrun <[email protected]>
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.

1 participant