-
Notifications
You must be signed in to change notification settings - Fork 102
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
Support for per-user configuration files and configuration inheritance #422
Comments
@jorisroovers you're the final decider here but my experience is that it is horribly confusing to have user config extend/override/blend with project configurations. If a user config is desirable to you (I don't think the value is there for adding and maintaining it), make it the fallback that's used if and only if the project config is missing |
To give a bit of context, we are currently in the process of selecting and configuring various tools to enforce consistent coding conventions across our company. Some tools have the option to configure things globally, some require to have their configuration files included in the repository. However, this makes it especially hard if you want to encourage adoption of new conventions, as it requires all configuration files to be updated in every repository. The feature I am proposing can be seen in other tools within the same domain as well:
In the case of gitlint, I would like to be able to enable conventional commits for all repositories, and specify the valid types (e.g. Please take this as a suggestion only, if you feel this behavior doesn't fit the project I'm perfectly fine with that. However I'd be happy to help if you consider this a useful feature. |
The request in #395 (adding I understand the usefulness of this (using it myself frequently). I’d need to think a bit more on what @sigmavirus24 said - I agree it adds more complexity for both users and gitlint maintenance. Arguably, there’s already quite a bit of complexity with gitlint’s existing configuration precedence: https://jorisroovers.com/gitlint/configuration/#configuration-precedence If we were to implement this, I’d definitely want to figure out the code design up front to ensure we do it right. Long story short, I’m inclined to do this, but would like to punt on this until I have more time so I can be involved in the actual implementation. |
So for what it's worth, Flake8 had user config. It's not dissimilar on it's behavior to gitlint. The user file had lowest precedence. The project config. Then CLI options. The problem we ran into was users expecting to see errors reported but not seeing that and then seeing them elsewhere (e.g., CI). The problem was always the user config file. It caused so much trouble for users that we accepted the pain of removing it. We will never add it back. It's not worth the headache. Like I said, the only thing that could have made that marginally less confusing would have been to ignore the user file if a project file was found. |
Ok, that’s definitely an interesting datapoint @sigmavirus24 - I think I’m undecided now. Thinking out loud (and documenting other solutions). Git solves this using [include]
path = subdir/gitconfig Note that its include logic is reversed in that it won’t read repository level Applied for gitlint, the repo-level # This is just regular gitlint [general] section, nothing special here
[general]
ignore=body-is-missing,T3
# This is the relevant section #############
# Include user config, overrides anything that came before it
[include]
path = ~/.gitlint
# Includes could be named to allow multiple of them
[include:myinclude]
path = ~/.gitlint2
# Includes could point to a local .gitlint-extra that is gitignored and/or symlinked by the user to their own .gitlint config
[include:extras]
path = .gitlint-extra
# One upside is that this could allow the user to specify whether to override of extend
[include:myextends]
path = .gitlint-extends
mode = extend # default = override
############################################
# Rest of gitlint file resumes here, overrides any includes before it
[title-max-length]
line-length=80 Of course, there’s a few major downsides/shortcomings:
From an implementation perspective this also is also non-trivial (not hard, just some work) as this is not something While I can see the usefulness of includes in general (this would also solve #395), I’m not sure this is the path forward (it also doesn’t fully solve the original question). I’m interested to get your feedback.
And of course, this problem always remains to some extend. I do think the explicit use of |
Hmm, using |
So traverse to endlessly even to Besides the horrible security posture, what happens then with submodules. If I'm in a submodule with a current gitlint config ostensibly root would default to false. That would apply the submodule config on top of the parent repo config. That would be confusing and surprising. |
@sigmavirus24 I am not sure I understand your line of reasoning regarding the attack vector. This would imply my system had been severely compromised, since normally I am in control of which files I put in what location. In that case I'd assume there are plenty of other, more straightforward ways of compromising the contents of locally checked-out git repositories. However, I am not a security researcher and hence do not claim to have any expertise in that domain. |
You don't install plugins for tools you use without reading in detail the source code? Do you read the full source every time you upgrade? There have been numerous documented issues where someone has taken over a maintenance of a package, bought the package name, or hijacked it and uploaded malicious code. If you use one, or you contribute to another project using one, you're opening your system up to arbitrary traversals as soon as that malicious version reaches you. Just because you think you know what you're using, doesn't mean you always will. You need to program defensively at every layer, that means tools like gitlint need to consider these scenarios and users should as well |
It would be nice to have a per-user configuration file (e.g. ~/.gitlint) to have a way to enforce specific rules across all projects.
The per-git-repository configuration should extend/override the settings of the per-user-configuration file. For instance, one might want to enforce rules for title length across all repositories, but might want to add additional rules on a per-repository basis.
Ideally, this could be done in a similar fashion to npm: https://docs.npmjs.com/cli/v8/configuring-npm/npmrc
Would be willing to contribute.
The text was updated successfully, but these errors were encountered: