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

Rename "pull" functions #194

Merged
merged 8 commits into from
Oct 24, 2024
Merged

Rename "pull" functions #194

merged 8 commits into from
Oct 24, 2024

Conversation

richfitz
Copy link
Member

This PR fixes #146, by renaming the location pull commands:

  • orderly_location_pull_metadata becomes orderly_location_metadata_fetch
  • orderly_location_pull_packet becomes orderly_location_pull

There's a bit of deprecation support, but I think that won't be needed for long.

Two small issues arising:

  • Do we prefer metadata_fetch or fetch_metadata?
  • The pull_metadata argument now does not make much sense; fixing this is a little more disruptive but we are uniquely placed to fix it after Ergonomic improvements for querying, part 2 #187 as people are updating anyway

@richfitz richfitz marked this pull request as ready for review October 23, 2024 15:47
@richfitz richfitz requested a review from M-Kusumgar October 23, 2024 15:47
@richfitz richfitz changed the title Gh 146 Rename "pull" functions Oct 23, 2024
Copy link
Contributor

@M-Kusumgar M-Kusumgar left a comment

Choose a reason for hiding this comment

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

awesome, i think i actually prefer orderly_location_fetch_metadata after youve brought it up, reads more like english

yh pull_metadata arg should probably be fetch_metadata, random thought, might be cool to replicate docker style pull arg so instead of boolean, we can have "always", "never" and "missing" so we automatically pull in missing ones on demand

@richfitz
Copy link
Member Author

yh pull_metadata arg should probably be fetch_metadata, random thought, might be cool to replicate docker style pull arg so instead of boolean, we can have "always", "never" and "missing" so we automatically pull in missing ones on demand

I've done the rename. The ternery arg here is probably not really the same as what we do though so I've left that as boolean but we can discuss that later if you think there's something I'm missing here

@richfitz richfitz merged commit 478e76e into main Oct 24, 2024
10 checks passed
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.

suggestion: change function name orderly_location_pull_metadata() to orderly_location_metadata_update()
2 participants