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

docs: Improve help message for ws restore command #1659

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

Conversation

jamilraichouni
Copy link
Contributor

Using this subcommand can easily fail, when user do not know that the tarfile should be a gzip compressed file with one top level directory named 'workspace'.

Using this subcommand can easily fail, when user do not know that the
tarfile should be a gzip compressed file with one top level directory
named 'workspace'.
Copy link

sonarcloud bot commented Jul 18, 2024

Copy link

codecov bot commented Jul 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.64%. Comparing base (92041c3) to head (1b59caf).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1659   +/-   ##
=======================================
  Coverage   80.64%   80.64%           
=======================================
  Files         185      185           
  Lines        6090     6090           
  Branches      678      678           
=======================================
  Hits         4911     4911           
  Misses       1030     1030           
  Partials      149      149           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jamilraichouni jamilraichouni marked this pull request as ready for review July 18, 2024 10:55
Comment on lines +196 to +197
"gzip compressed file with one top level "
"directory named 'workspace'"
Copy link
Member

@MoritzWeber0 MoritzWeber0 Jul 18, 2024

Choose a reason for hiding this comment

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

Suggested change
"gzip compressed file with one top level "
"directory named 'workspace'"
"tarfile generated by the backup CLI command"

I wouldn't support manually created tarfiles officially, there is a lot more that can go wrong. The purpose of the command is to restore backups done by the CLI itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd rather keep it as proposed. I can imagine scenarios where people

1
backup a workpace

2
modify the backup

3
compress the modified backup

4
restore the packed workspace

The CLI can deal with a manually archived workspace folder. This is good and useful. It just lacks documentation here.

Copy link
Member

Choose a reason for hiding this comment

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

Accepting a custom archive would mean that we also need proper validation in the restore interface to provide a correct error message in case of wrongly packed archives. Instead, I'd propose to add two new CLI functions: pack and unpack. Unpack takes the archive and unpacks it into a target directory. Pack takes the directory and packs it again.

@MoritzWeber0 MoritzWeber0 marked this pull request as draft August 2, 2024 10:41
@jamilraichouni jamilraichouni marked this pull request as ready for review September 2, 2024 19:00
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.

2 participants