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

Rename "rpm" command to "db" #64

Merged
merged 2 commits into from Dec 2, 2014
Merged

Rename "rpm" command to "db" #64

merged 2 commits into from Dec 2, 2014

Conversation

mbarnes
Copy link
Contributor

@mbarnes mbarnes commented Dec 1, 2014

Fixes issue #22 + some cleanup work on rpmostree-builtin-db.c

Note the "rpm-ostree rpm/db version" command is apparently broken atm, but I believe it's unrelated to these changes.

@cgwalters
Copy link
Member

With patches like this that involve lots of "code motion" and some cleanup, can you highlight what major semantic changes were made, if any?

For cleanup I see moving _rpmostree_util_get_commit_hashes() into -util.c. Anything else?

The patch looks good to me though.

@mbarnes
Copy link
Contributor Author

mbarnes commented Dec 1, 2014

Sure, I split the "db diff/list/version" subcommands into separate files:

rpmostree-db-builtin-diff.c
rpmostree-db-builtin-list.c
rpmostree-db-builtin-version.c

Now rpmostree-builtin-db.c just does option parsing and command dispatching, like rpmostree-builtin-compose.c.

I moved the support functions for the "db" subcommands to rpmostree-rpm-util.[ch]. Some functions that were "static" before are now exposed in the header file, not that it really matters.

For _rpmostree_util_get_commit_hashes(), there's nothing rpm-specific about it so I moved it to rpmostree-util.c. Could potentially move that one into libostree someday.

@cgwalters
Copy link
Member

LGTM, feel free to squash and commit, thanks!

Eliminates some confusion between "rpm-ostree rpm" (or "atomic rpm")
commands versus actual "rpm" commands.

The "rpm" subcommand is retained as a hidden alias for the "db"
subcommand for backward-compatibility.  It is not listed in --help
output.

Fixes #22
As a followup to renaming the "rpm" command to "db", split the "db"
subcommands into separate source files in the style of "ostree admin"
and "rpm-ostree compose".

Also create rpmostree-rpm-util.[ch] as a place for common rpm-related
functions needed by the "db" subcommands.

No intentional functional changes here, just a bunch of copy-n-paste
and minor cleanup.
mbarnes added a commit that referenced this pull request Dec 2, 2014
@mbarnes mbarnes merged commit 99f9bc3 into coreos:master Dec 2, 2014
cgwalters added a commit to cgwalters/rpm-ostree that referenced this pull request Dec 4, 2014
This broke with coreos#64 I
believe.

The argument parsing here was rather hairy, and I think this patch
clarifies things, in addition to fixing the bug.
@mbarnes mbarnes deleted the rpm2db branch November 16, 2015 21:26
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.

None yet

2 participants