Adding recaptool and better path handling. #79
Conversation
There was a problem hiding this comment.
It's looking good to me - all permissions on the files are correct, all files install and uninstall cleanly and are where they are expected to be.
Knowing this was traditionally a RHEL/CentOS install, I tested specifically on a minimal Ubuntu 16 VM adding nothing more than 'make' to the base image and it's as expected, ready for packagers to build as non-root user.
| @echo "Installing configuration..." | ||
| @install -Dm0644 src/recap.conf $(SYSCONFDIR)/recap |
There was a problem hiding this comment.
My only question is this one -- why are we stripping the dot-conf off the end of the file when installing it? Is this part of expected operation?
There was a problem hiding this comment.
oops tagged one line in the PR too high, I meant the next line down sorry.
There was a problem hiding this comment.
according to https://github.com/rackerlabs/recap/blob/master/src/recap#L68 That's the proper name of the config file
edit: logic @ https://github.com/rackerlabs/recap/blob/master/src/recap#L643
There was a problem hiding this comment.
sigh OK - changing that would break a simple upgrade, we should really be naming that file recap.conf. :(
There was a problem hiding this comment.
So yes, as @thebwt points out is what's used on recap. I chase whether or not this is a good practice and could've swear I found a source that would make a distinction on that matter, something like this (from what I remember):
- /etc/project - OK when there is no other config file
- /etc/project.conf - OK when there is a directory file for that project and requires more configs, e.g. /etc/project/
- /etc/project/ - From above, when needed to include more configs
- /etc/project.d/ - Similar as above but more explicit, when needed to include more configs
Don't remember if it was a best practice for packaging or something similar but that made me think it was not completely wrong. But truth is I don't find at the moment anything to support that and since I don't see any reference to this in the FSH for /etc/ this opens the question to. What's the right thing to do?
For now is clear that without a new release a change like this may not be recommended, but definitely something to take into consideration.
There was a problem hiding this comment.
I agree, this PR is not the place for a change like that. Though I am not taking a stance on the merits of the proposal in any way, just open an issue/pr to fix it.
There was a problem hiding this comment.
alright, I've created #80 to keep the discussion of the config file, since there is no other problem with the Makefile I'll wait a couple of days then proceed to merge it.
|
Merging this one. |
No description provided.