-
Notifications
You must be signed in to change notification settings - Fork 56
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
Warnings for empty folders triggered when using "elvis git-branch" #196
Comments
Hi, @cdyb-kivra. Thanks for the report. I'm not sure what "using the git-branch subcommand to elvis" means 😕, though. In any case, when the warning is issued, is it correct? (i.e. are the folders empty or non-existing?) It was, indeed, not a concern of mine when developing it to assume different behaviours using branches, but I can revisit that. Since it's a warning, and not an error (we did discussed it being an error, but didn't go that route) what would you suggest we do differently? |
The git-branch subcommand is used to only run elvis on those files that have been changed compared to a given git branch. So, for example, my I haven't looked at how this is implemented, but offhand it feels like the check that triggers the warning should be applied before the git-branch filtering is done. Or, possibly, that the warning should just be suppressed entirely when the git-branch subcommand is running. |
I agree it's no good. I didn't know about I'm pulling @elbrujohalcon to this issue, since this'll probably also include changes to
This is the bit that seems to need some kind of tweaking in the code. |
It's been ages since I last saw |
At the company I work for (Kivra), we use |
Sure. Let's wait for @jfacorro's input, then. We either have to silence elvis_core via an API variable, an option or, ... another thing (?) |
The warning introduced in commit 1cfbd98 gets triggered when using the git-branch subcommand to elvis, and the branch being worked on lacks changed files in some configured directories. That's not very useful, and I assume was not intended.
The text was updated successfully, but these errors were encountered: