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

tool/update_tool: new tool to explore db updates #42

Merged
merged 1 commit into from
Sep 8, 2022

Conversation

lavocatt
Copy link
Contributor

@lavocatt lavocatt commented Aug 30, 2022

Db updates are tricky to review. This tools brings some utility
functions to try to simplify the process. It gives you access to 3
commands: Ls, Diff and Report

List the changed DB entry and know if the manifest or the image info was
updated easily. The tool also ignores by default the volatiles UUID so
you are not polluted by that during the review process.

Diff the changed files, either all of them one after the other or a
specific set given by the user. It will open a difftool (vimdiff by
default) to diff either the manifest or the image-info (or both if they
both changed).

Report the change as an ascii table with color codes (by default) or as
a github TODO list to be integrated within the github PR.

report generation

Terminal report

Peek updatetool_report

Github report

Peek updatetool_report_gh

Listing changed DB entries

Peek updatetool_ls

Diff all or a list of files

Peek updatetool_diff

@ondrejbudai
Copy link
Member

May I ask you for a quick example of the output? That would be very valuable to everyone having a look at this PR. :)

@lavocatt
Copy link
Contributor Author

lavocatt commented Sep 1, 2022

May I ask you for a quick example of the output? That would be very valuable to everyone having a look at this PR. :)

Good idea! I'll record my screen using it with peek and include the gifs inside the PR description and in the README also :)

Copy link
Member

@achilleas-k achilleas-k left a comment

Choose a reason for hiding this comment

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

Added some comments in the diff function that apply to all places where the same code and logic is used.

tools/update_tool Outdated Show resolved Hide resolved
tools/update_tool Show resolved Hide resolved
Copy link
Member

@ondrejbudai ondrejbudai left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the demos, they look really sleek. Some suggestions inline to make the code nicer.

@supakeen May I ask for your opinion on using some kind of templating? I'm still not sure if it improved readability, but I'm just not a fan of formatting text imperatively.

tools/update_tool Outdated Show resolved Hide resolved
tools/update_tool Outdated Show resolved Hide resolved
tools/update_tool Outdated Show resolved Hide resolved
tools/update_tool Outdated Show resolved Hide resolved
tools/update_tool Outdated Show resolved Hide resolved
tools/update_tool Outdated Show resolved Hide resolved
tools/update_tool Outdated Show resolved Hide resolved
tools/update_tool Outdated Show resolved Hide resolved
@supakeen
Copy link
Member

supakeen commented Sep 2, 2022

@ondrejbudai I'll take a look later this morning, thanks.

tools/update_tool Outdated Show resolved Hide resolved
tools/update_tool Show resolved Hide resolved
tools/update_tool Outdated Show resolved Hide resolved
tools/update_tool Outdated Show resolved Hide resolved
Copy link
Member

@supakeen supakeen left a comment

Choose a reason for hiding this comment

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

I like the way the formatting is structured now much more.

tools/update_tool Outdated Show resolved Hide resolved
tools/update_tool Outdated Show resolved Hide resolved
Copy link
Member

@ondrejbudai ondrejbudai left a comment

Choose a reason for hiding this comment

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

Looks much better, thanks! There are still some unresolved comments but they should be easy to fix. :)

tools/update_tool Outdated Show resolved Hide resolved
tools/update_tool Outdated Show resolved Hide resolved
Copy link
Member

@ondrejbudai ondrejbudai left a comment

Choose a reason for hiding this comment

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

One forgotten function, otherwise this is fine. :)

tools/update_tool Outdated Show resolved Hide resolved
Db updates are tricky to review. This tools brings some utility
functions to try to simplify the process. It gives you access to 3
commands: Ls, Diff and Report

List the changed DB entry and know if the manifest or the image info was
updated easily. The tool also ignores by default the volatiles UUID so
you are not polluted by that during the review process.

Diff the changed files, either all of them one after the other or a
specific set given by the user. It will open a difftool (vimdiff by
default) to diff either the manifest or the image-info (or both if they
both changed).

Report the change as an ascii table with color codes (by default) or as
a github TODO list to be integrated within the github PR.
@lavocatt lavocatt merged commit 799deea into osbuild:main Sep 8, 2022
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