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 KDB commands #3023

Merged

Conversation

@PhilippGackstatter
Copy link
Contributor

commented Oct 2, 2019

  • Prefix kdb info, check and list with plugin-
  • Rename meta commands to meta-rm, meta-set, etc.
  • Remove kdb fstab

fixes #265

Basics

Check relevant points but please do not remove entries.
Do not describe the purpose of this PR in the PR description but:

  • Short descriptions should be in the release notes (added as entry in
    doc/news/_preparation_next_release.md which contains _(my name)_)
    Please always add something to the the release notes.
  • Longer descriptions should be in documentation or in design decisions.
  • Describe details of how you changed the code in commit messages
    (first line should have module: short statement syntax)
  • References to issues, e.g. close #X, should be in the commit messages.

Checklist

Check relevant points but please do not remove entries.
For docu fixes, spell checking, and similar none of these points below
need to be checked.

  • I added unit tests for my code
  • I fixed all affected documentation
  • I added code comments, logging, and assertions as appropriate (see Coding Guidelines)
  • I updated all meta data (e.g. README.md of plugins and METADATA.ini)
  • I mentioned every code not directly written by me in THIRD-PARTY-LICENSES

Review

Reviewers will usually check the following:

Labels

  • Add the "work in progress" label if you do not want the PR to be reviewed yet.
  • Add the "ready to merge" label if the build server is happy and also you
    say that everything is ready to be merged.
@markus2330

This comment has been minimized.

Copy link
Contributor

commented Oct 2, 2019

Thank you for creating the PR!

@PhilippGackstatter

This comment has been minimized.

Copy link
Contributor Author

commented Oct 5, 2019

The jenkins build fails with this

dh_install --list-missing
dh_install: Cannot find (any matches for) "usr/share/man/man1/kdb-check.1" (tried in "." and "debian/tmp")
dh_install: elektra-bin missing files: usr/share/man/man1/kdb-check.1
dh_install: Cannot find (any matches for) "usr/share/man/man1/kdb-fstab.1" (tried in "." and "debian/tmp")
dh_install: elektra-bin missing files: usr/share/man/man1/kdb-fstab.1
...

I renamed kdb-check to kdb-plugin-check, both the doc/help markdown file and the man page itself. I can't find any explicit reference to those files, so why is that file still searched for?

@sanssecours

This comment has been minimized.

Copy link
Member

commented Oct 5, 2019

I renamed kdb-check to kdb-plugin-check, both the doc/help markdown file and the man page itself. I can't find any explicit reference to those files, so why is that file still searched for?

The problem here is the dreaded debian branch. You probably have to update some of the Debian package description files in the folder debian. Otherwise building the Debian packages will not work.

@PhilippGackstatter

This comment has been minimized.

Copy link
Contributor Author

commented Oct 5, 2019

Thank you! Since this is a different branch, what's the best way to go about this? Do I submit a separate PR that includes all of the changes from this PR plus the debian specific changes?

@markus2330

This comment has been minimized.

Copy link
Contributor

commented Oct 5, 2019

You are allowed to directly push commits to the debian branch to fix such problems (add/remove files). The problems with the man pages is hopefully already fixed with e78d603

@markus2330

This comment has been minimized.

Copy link
Contributor

commented Oct 5, 2019

jenkins build libelektra please

@PhilippGackstatter

This comment has been minimized.

Copy link
Contributor Author

commented Oct 5, 2019

The problems with the man pages is hopefully already fixed with e78d603

Thank you!

@markus2330

This comment has been minimized.

Copy link
Contributor

commented Oct 6, 2019

Looks good, all tests passed. Is this ready to merge?

@PhilippGackstatter

This comment has been minimized.

Copy link
Contributor Author

commented Oct 6, 2019

Yes, it's ready to merge then.

@markus2330 markus2330 merged commit 704812a into ElektraInitiative:master Oct 6, 2019
11 of 13 checks passed
11 of 13 checks passed
LGTM analysis: JavaScript No code changes detected
Details
LGTM analysis: Python No code changes detected
Details
LGTM analysis: C/C++ No new or fixed alerts
Details
continuous-integration/jenkins/pr-merge This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
restyled No differences
Details
🍎 Clang Task Summary
Details
🍎 Clang ASAN Task Summary
Details
🍎 FULL Task Summary
Details
🍎 MMap Task Summary
Details
🔗 Check Task Summary
Details
😈 FreeBSD 11 Task Summary
Details
😈 FreeBSD 12 Task Summary
Details
@markus2330

This comment has been minimized.

Copy link
Contributor

commented Oct 6, 2019

Thank you, great work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.