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

flowctl draft author should prune specs that aren't in the local sources #865

Closed
psFried opened this issue Dec 29, 2022 · 2 comments · Fixed by #906
Closed

flowctl draft author should prune specs that aren't in the local sources #865

psFried opened this issue Dec 29, 2022 · 2 comments · Fixed by #906
Labels
flowctl Issues related to the user facing CLI

Comments

@psFried
Copy link
Member

psFried commented Dec 29, 2022

Say you run the following steps:

  • flowctl catalog draft --name acmeCo/foo
  • flowctl draft develop
  • Rename acmeCo/foo => acmeCo/bar in the local spec files
  • flowctl draft author
  • flowctl draft publish 💥 your draft includes both acmeCo/foo and acmeCo/bar

There's not an obvious (to me) best way to handle this, but any of these options seem significantly better to me:

  • draft author clears all specs from the draft before adding new ones. This way your existing acmeCo/foo remains untouched.
  • draft author changes all specs in the draft to null (triggering the deletion of the live specs upon publication) before adding new ones. This way your existing acmeCo/foo will be deleted by the publication.
  • draft author accepts an argument similar to --existing abort|keep|overwrite|merge-spec from catalog pull-specs, which explicitly says what to do with an existing draft specs.

Given that there's now, with the introduction of catalog pull-specs|test|publish, relatively few reasons to use draft at all, my inclination is to have draft author simply clear all specs from the draft before adding new ones. It's a super easy change to make, and addresses the biggest foot gun. It would also be easy to add a flag to allow opting in to the current behavior.

@psFried psFried added the flowctl Issues related to the user facing CLI label Dec 29, 2022
@jgraettinger
Copy link
Member

my inclination is to have draft author simply clear all specs from the draft before adding new ones

I think we can just do this, without flagging for the prior behavior.

@rijnhard
Copy link

I think we can just do this, without flagging for the prior behaviour.

I tend to agree, You are working from a code base anyway, if you removed something and need to re-add it for whatever reason then it's up to your VCS to assist there.

psFried added a commit that referenced this issue Jan 31, 2023
Resolves #865

Updates the `author` subcommand to delete all existing draft specs prior
to inserting the new ones. This is to better match user expectations
when running `flowctl draft author` after renaming or deleting local
specs.
psFried added a commit that referenced this issue Feb 1, 2023
Resolves #865

Updates the `author` subcommand to delete all existing draft specs prior
to inserting the new ones. This is to better match user expectations
when running `flowctl draft author` after renaming or deleting local
specs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flowctl Issues related to the user facing CLI
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants