Skip to content

Rename vast dump to vast show #2686

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

Merged
merged 3 commits into from
Nov 10, 2022
Merged

Conversation

dominiklohmann
Copy link
Member

@dominiklohmann dominiklohmann commented Nov 8, 2022

We're planning to do more with the mostly internal command in the near future, and to quote @lava on the matter:

I wanna take the opportunity to suggest renaming vast dump to vast show, since dump sounds like some kind of backup or debug command.

I very much agree with that sentiment. This change implements the transition without any deprecation period as the command is not even documented currently and I do not anticipate anyone actually depending on it.

📝 Reviewer Checklist

Review this pull request by ensuring the following items:

  • All user-facing changes have changelog entries
  • User-facing changes are reflected on vast.io

@dominiklohmann dominiklohmann requested a review from a team November 8, 2022 00:24
We're planning to do more with the mostly internal command in the near
future, and to quote @lava on the matter:

> I wanna take the opportunity to suggest renaming `vast dump` to `vast
> show`, since `dump` sounds like some kind of backup or debug command.

I very much agree with that sentiment. This change implements the
transition without any deprecation period as the command is not even
documented currently and I do not anticipate anyone actually depending
on it.
@dominiklohmann dominiklohmann force-pushed the story/sc-32781/dump-to-show branch from eb0377d to a53aef2 Compare November 8, 2022 00:25
@tobim tobim force-pushed the story/sc-38757-arrow-10-support branch from 961c68d to 027ba25 Compare November 8, 2022 15:23
@mavam
Copy link
Member

mavam commented Nov 9, 2022

Not strictly required for this PR, but upcoming documentation will go to https://vast.io/docs/use/introspect, right?

@dominiklohmann
Copy link
Member Author

Not strictly required for this PR, but upcoming documentation will go to https://vast.io/docs/use/introspect, right?

That I will leave up to you. Just waiting for the Arrow PR to be merged now so CI gets fixed on this one.

Base automatically changed from story/sc-38757-arrow-10-support to master November 9, 2022 12:40
@dominiklohmann dominiklohmann merged commit 8bf2acc into master Nov 10, 2022
@dominiklohmann dominiklohmann deleted the story/sc-32781/dump-to-show branch November 10, 2022 07:43
@tobim
Copy link
Member

tobim commented Nov 10, 2022

I believe this PR was missing a git mv taxonomy-dump show-taxonomies?

@dominiklohmann
Copy link
Member Author

I believe this PR was missing a git mv taxonomy-dump show-taxonomies?

Already fixed in #2638 directly. We also just noticed. This got auto-merged accidentally while rewriting the branch protections for #2655.

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.

4 participants