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

Warnings for empty folders triggered when using "elvis git-branch" #196

Open
cdyb-kivra opened this issue May 25, 2021 · 6 comments
Open

Comments

@cdyb-kivra
Copy link

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.

@paulo-ferraz-oliveira
Copy link
Collaborator

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?

@cdyb-kivra
Copy link
Author

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 elvis.config points at three directories, src, test and include. Most of the time I work on a feature branch and change files in src and test. I then run the elvis git-branch command to get it to check only those files I have actually edited (compared to our main branch). With the latest elvis version, I then always get a warning about there being no files in include and that I should update my configuration. Which is not true. Since it's a warning, it's not really a problem, but it is annoying and it teaches to ignore warnings from elvis.

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.

@paulo-ferraz-oliveira
Copy link
Collaborator

it is annoying and it teaches to ignore warnings from elvis

I agree it's no good.

I didn't know about elvis git-branch. I use elvis only as a plug-in, which is why I didn't consider that "edge" case.

I'm pulling @elbrujohalcon to this issue, since this'll probably also include changes to elvis.

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.

This is the bit that seems to need some kind of tweaking in the code. elvis is "aware" of elvis_core but not the other way around, though they should play nice.

@elbrujohalcon
Copy link
Member

elbrujohalcon commented May 26, 2021

It's been ages since I last saw git-branch anywhere. I think that, instead of myself, you should pull @jfacorro here, @paulo-ferraz-oliveira.
Sorry, but I can help as much or even less than you on this topic.

@cdyb-kivra
Copy link
Author

At the company I work for (Kivra), we use elvis git-branch as part of our CI/CD pipeline. So it's not just me personally :-)

@paulo-ferraz-oliveira
Copy link
Collaborator

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 (?)

@elbrujohalcon elbrujohalcon modified the milestones: 2.0.0, eventually 🙄 May 26, 2022
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

No branches or pull requests

3 participants