-
Notifications
You must be signed in to change notification settings - Fork 576
Allow Multiple Dotenv Files #2682
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
base: master
Are you sure you want to change the base?
Conversation
I understand @casey's pretty busy right now so if anyone else wants to take a look at this and give me any feedback, feel free. In the meantime I've been test driving this locally and it's been working well |
@casey just pinging you because I'd love to get this feature in, and also if you could do whatever you need to do to get CI to run that'd be helpful because I could try and fix any errors that come up |
+1 for this feature PR pretty please |
That would be very useful. here my use case:
in sort, that would require to merge based on the environment var name, with the left-most precedence. |
@gsemet usually I've seen |
@casey I'd love to get this in soon, it would enable the very common workflow of one |
Looks like there's some conflicts from the dotenv-override setting that got merged, I'll try and fix them today |
@casey can you approve the workflow run, and ping me when you do? |
@kwuite are you a maintainer and can you approve the workflow run? Otherwise you could install this branch locally to try it out and let me know if you run into any issues: cargo install just --git https://github.com/mikeyhew/just --branch multiple-dotenv-files |
@mikeyhew , my apologies, I am not a maintainer of this project. I am currently developing a DevOps CLI interface for my startup, ASD Engineering, and my tool is designed to work like a git submodule. I do not want my .asd code to get mixed up with project specific environment variables. The environment variables are consumed from multiple folders, with module-specific .env files stored in a workspace. However, all these values now need to be merged into the user repository .env file, because I’m not allowed to import them from two separate locations (the user’s repo and my .asd/workspace/.env file). I wrote some decently hacky code to get this working but I would love to get rid of it and use a pure just approach. A few weeks back I found your PR and was hoping for a merge but the way it looks this code will not get merged any time soon. I would be happy to review your code, test it, validate it against my own (currently private .asd) implementation, and if everything works well, build and publish a binary via GitHub similar to the original just creator. Would that help you in any way? Allthough I am still calculating myself whether this is the route I want to take🤔. |
@kwuite I'd love it if you'd be willing to test it out, provide feedback etc. Just keep in mind there's still no guarantee this gets merged in its current state, I'm not sure what @casey's stance on it is since I haven't heard from him yet. It looks like he did enable the CI run though so that's a good sign. |
@mikeyhew I have schedule 2 hours for testing tomorrow afternoon. Will provide feedback in this channel. Have a great day👍 |
This adds support for multiple dotenv files by updating the dotenv-filename and dotenv-path settings to support an array of strings instead of just a single string. Files are loaded in the order specified in the array, and variables in later files override variables in earlier files.
Some edge cases to point out:
We could add some checks for some of these edge cases if they are too odd and disallow them with an error. But I get the sense from looking at the code and tests surrounding dotenv loading that the philosophy in Just is to not worry too much about disallowing weird edge cases.
Note that there is one potential breaking change here: this updates the dotenv_filename and dotenv_path settings to be
Vec<String>
andVec<PathBuf>
instead ofOption<String>
andOption<PathBuf>
, so when you calljust --dump --dump-format=json
, they will be output as arrays instead of as a single string ornull
. If we need to guarantee backward compatibility for json dumps then it's possible to change this to output asnull
or a single string if there are less than 2 entries in the array, at the expense of making the output more complicated to parse.See this comment for more details on the design for this PR.
Fixes #1748