Skip to content
This repository has been archived by the owner on Aug 7, 2023. It is now read-only.

Interpolate env var in conf path #297

Merged

Conversation

SpainTrain
Copy link
Member

  • 🆕 Interpolate environment variables in the provided path for the eslint config file

Closes #291

Review on Reviewable

* Interpolate environment variables in the provided path for the eslint config file

Closes AtomLinter#291
@steelbrain
Copy link
Contributor

While what you did looks nice, I wouldn't trust a module who is not popular and we don't even know it's author, what if he doesn't follow semver and breaks the API?

I would highly recommend we pin the version of it or find an alternative package that is more trust worthy

@JohnMurga
Copy link
Contributor

The entire code for the module appears to be 15 lines :-)
https://github.com/LinusU/node-resolve-env/blob/master/index.js

@SpainTrain
Copy link
Member Author

I really doubt that will be an issue given that the author has many packages (https://www.npmjs.com/~linusu) and is a member on @expressjs (https://github.com/LinusU)

@LinusU, can you comment on following semver in your projects?

@SpainTrain
Copy link
Member Author

RE: LOC - sindresorhus/ama#10 (comment)

@JohnMurga
Copy link
Contributor

@SpainTrain ... Agreed ... I was thinking more about risk in relation to (hidden) complexity ...
Excluding white space it is 11 lines, clean and concise 😄

@SpainTrain
Copy link
Member Author

oh gotcha! 👍

@LinusU
Copy link

LinusU commented Nov 18, 2015

I promise to follow semver, which I do with all of my modules :)

I also fullheartedly agree with sindresorhus/ama#10 (comment) and I'm usually very fast at responding to bug reports.

@SpainTrain
Copy link
Member Author

Thanks for responding!

strangers-npm

@steelbrain are we good to merge?

steelbrain added a commit that referenced this pull request Nov 18, 2015
@steelbrain steelbrain merged commit 5976411 into AtomLinter:master Nov 18, 2015
@LinusU
Copy link

LinusU commented Nov 18, 2015

Awesome 👍

@steelbrain
Copy link
Contributor

@LinusU Thank you for a nice package! 👍

@Arcanemagus
Copy link
Member

Published in v5.2.1.

@JohnMurga
Copy link
Contributor

Thanks all 👍
However, I had to extend this to the "eslintrcPath" with this #305 tiny patch.
Now it works for me in combination with setting the the disableWhenNoEslintrcFileInPath config.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants