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

CLI: make snapshot name requiered in creating volume snapshots #17958

Merged
merged 3 commits into from
Aug 4, 2023

Conversation

yazdan
Copy link
Contributor

@yazdan yazdan commented Jul 16, 2023

  • Make snapshot name argument requiered
  • Update documentation and helps

@hashicorp-cla
Copy link

hashicorp-cla commented Jul 16, 2023

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@jrasell jrasell left a comment

Choose a reason for hiding this comment

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

Hi @yazdan and thanks for raising this PR. I have added some inline comments and suggestions; please let me know if you have any follow up questions.

It would be good to add a changelog entry for this also. The make cl command can guide you through this.

command/volume_snapshot_create.go Outdated Show resolved Hide resolved
command/volume_snapshot_create.go Outdated Show resolved Hide resolved
command/volume_snapshot_create.go Outdated Show resolved Hide resolved
@jrasell jrasell self-assigned this Jul 24, 2023
@jrasell jrasell moved this from Needs Triage to In Progress in Nomad - Community Issues Triage Jul 24, 2023
- Make snapshot name argument requiered
- Update documentation and helps
@yazdan yazdan force-pushed the make-volume-snapshot-name-required branch from 65ba808 to f274147 Compare July 29, 2023 22:52
@yazdan
Copy link
Contributor Author

yazdan commented Jul 29, 2023

@jrasell done, please review again.

Copy link
Member

@jrasell jrasell left a comment

Choose a reason for hiding this comment

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

LGTM and thanks @yazdan! I have made a suggestion to the changelog entry, once this is updated I will get this merged and backported.

.changelog/17958.txt Outdated Show resolved Hide resolved
Co-authored-by: James Rasell <jrasell@users.noreply.github.com>
@yazdan
Copy link
Contributor Author

yazdan commented Aug 4, 2023

@jrasell done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.4.x backport to 1.4.x release line backport/1.5.x backport to 1.5.x release line backport/1.6.x backport to 1.6.x release line
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants