Skip to content

added one message at the beginning of dryRunPurge#106

Merged
northtyphoon merged 2 commits intoAzure:mainfrom
wangxiaoxuan273:improve-dry-run-output
Apr 29, 2022
Merged

added one message at the beginning of dryRunPurge#106
northtyphoon merged 2 commits intoAzure:mainfrom
wangxiaoxuan273:improve-dry-run-output

Conversation

@wangxiaoxuan273
Copy link
Member

@wangxiaoxuan273 wangxiaoxuan273 commented Apr 27, 2022

Purpose of the PR
#107 Improved the output message in dry run mode. Added one message making clear that nothing will be deleted, and changed the wording for dry run output.

Output after the change:

$ ./bin/acr purge \
--registry wxxfirstcontainerregistry.azurecr.io \
--filter "app:^hello.*" \
--ago 10d 

Number of deleted tags: 0
Number of deleted manifests: 0

$ ./bin/acr purge \
--registry wxxfirstcontainerregistry.azurecr.io \
--filter "app:^hello.*" \
--ago 10d \
--dry-run

DRY RUN: The following output shows what WOULD be deleted if the purge command was executed. Nothing is deleted.

Number of tags to be deleted: 0
Number of manifests to be deleted: 0

Copy link
Member

@shizhMSFT shizhMSFT left a comment

Choose a reason for hiding this comment

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

Instead of a claim at the very beginning, how about changing all the wordings for dry run?

For example,

  • Tags to be deleted for repository: ...
  • Number of tags to be deleted: ...
  • Number of manifests to be deleted: ..

The above wording sounds less scary and can be distinguished from real purge easily.
Note: Lots of people read the logs from the bottom line to the top.

Copy link
Member

@shizhMSFT shizhMSFT left a comment

Choose a reason for hiding this comment

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

LGTM. It would be better to update the PR description with some tags / manifest to be deleted instead of 0s.

Copy link
Member

@northtyphoon northtyphoon left a comment

Choose a reason for hiding this comment

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

LGTM

@northtyphoon northtyphoon merged commit 5c58deb into Azure:main Apr 29, 2022
@wangxiaoxuan273 wangxiaoxuan273 deleted the improve-dry-run-output branch April 29, 2022 08:42
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.

3 participants