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

config file discovery #235

Closed
delfick opened this issue Sep 26, 2019 · 7 comments
Closed

config file discovery #235

delfick opened this issue Sep 26, 2019 · 7 comments

Comments

@delfick
Copy link

delfick commented Sep 26, 2019

Is your feature request related to a problem? Please describe.
I want to be able to run revive from my text editor without having to make my text editor find revive.toml to use.

Describe the solution you'd like
Have revive look up directories until it finds a revive.toml rather than only looking in home.

Describe alternatives you've considered
I can make vim find a revive.toml to use, but I'd rather rely on revive itself for that.

@mgechev
Copy link
Owner

mgechev commented Sep 27, 2019

My concern would be for systems where the file look up is expensive (for example, at Google we're using one). This may delay the initial start of revive a lot depending on how deeply nested is the current working directory.

@delfick
Copy link
Author

delfick commented Sep 27, 2019

perhaps introduce a cli argument to disable config discovery?

I imagine for most people, file lookup isn't expensive.

@mgechev
Copy link
Owner

mgechev commented Oct 17, 2019

I can make vim find a revive.toml to use, but I'd rather rely on revive itself for that.

Maybe that's good enough? I'm trying to keep the configuration minimalistic and do not introduce any further configuration options/logic when it could be achieved with external tools.

I'll keep the issue open in case this is a pain that many people want to resolve.

@chavacava
Copy link
Collaborator

Another concern with auto discovery is that you do not know precisely which configuration you are actually using (if you known which conf you are "auto discovering" then why not just point it explicitly?)

@delfick
Copy link
Author

delfick commented Oct 17, 2019

But it already finds a config in a home directory, which means developers can get different results from revive on their different computers based on whether they have one in home directory or not, whereas if one is in the repository, then they will have consistent results if they just say revive.

I'd also argue that auto finding configuration by looking up folders is a common pattern and less surprising than it not doing that.

@mgechev
Copy link
Owner

mgechev commented Oct 18, 2019

But it already finds a config in a home directory, which means developers can get different results from revive on their different computers based on whether they have one in home directory or not, whereas if one is in the repository, then they will have consistent results if they just say revive.

I'm more leaning towards dropping this feature.

I'd also argue that auto finding configuration by looking up folders is a common pattern and less surprising than it not doing that.

This is more common for dynamic languages, I'd say. In Node.js it's a common practice. I'm not confident it's a good one.

@mgechev
Copy link
Owner

mgechev commented Nov 10, 2019

Let's close this feature request for now and revisit if it's in a higher demand.

@mgechev mgechev closed this as completed Nov 10, 2019
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