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

Follow log directory creation pattern from other providers #132

Conversation

miha-plesko
Copy link
Contributor

With this commit we further improve log directory creation now that it's missing from git. This approach is better than what we had before because

a) it uses absolute path and will work regardless $PWD
b) it's executed everytime bin/setup is run, not just upon
spec/manageiq link creation

Copied from: ManageIQ/manageiq-providers-kubernetes#273

/cc @cben @tadeboro

With this commit we further improve `log` directory creation
now that it's missing from git. This approach is better than
what we had before because

a) it uses absolute path and will work regardless $PWD
b) it's executed everytime bin/setup is run, not just upon
   spec/manageiq link creation

Signed-off-by: Miha Pleško <miha.plesko@xlab.si>
@miq-bot
Copy link
Member

miq-bot commented Aug 21, 2018

Checked commit miha-plesko@1263801 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
0 files checked, 0 offenses detected
Everything looks fine. 👍

@tadeboro
Copy link
Contributor

I feel that this solution does not solve anything. If spec/manageiq folder already exists, it means that bin/setup already cloned it and ensured that log folder is present (this is what current solution does).

If developer manages spec/manageiq contents manually, then I would argue that creating a log folder automatically makes little sense (and may even annoy grumpy developers like me ;).

And about that relative/absolute path: git clone is already done using relative path, so no harm in using relative path there.

@miha-plesko
Copy link
Contributor Author

I must disagree, action "create empty log directory unless exist" is reasonable thing to do in bin/setup script. If one find it annoying that it automatically creates things for you, then he shouldn't call it in first place :P

@tadeboro
Copy link
Contributor

I have a problem that latest solution can create it on the other side of the file system in case of symlinked spec/manageiq. I really do not want something from /a/b/c/d messing around with /a/b/e/f.

@cben
Copy link

cben commented Aug 21, 2018

good point (though bin/setup under a provider causes stuff to be written to files under spec/manageiq/log/ anyway).
I don't really care which way we do it, waiting for gitter discussion to settle...

@tadeboro
Copy link
Contributor

The stuff that is placed in into spec/manageiq/log folder is created by code in the core repo. But yeah, I am also sitting with a popcorn on my lap and waiting for things to settle down;)

@miha-plesko
Copy link
Contributor Author

Closing as issue has been resolved in core repo via ManageIQ/manageiq#17886 so providers no longer need to bother with it.

@miha-plesko miha-plesko deleted the improve-log-directory-creation branch August 31, 2018 06:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants