read additional registry URLs from DUB_REGISTRY env var #1173
Conversation
MartinNowak
commented
Jun 26, 2017
MartinNowak
commented
- multiple URLs separated by semicolon
|
Thanks for your pull request, @MartinNowak! |
MartinNowak
force-pushed
the
env_registry_urls
branch
from
66f00a6
to
967436c
Compare
|
Can you change the settings of Coveralls to not leave comments btw? |
Done. |
source/dub/commandline.d
Outdated
| @@ -257,6 +257,7 @@ struct CommonOptions { | |||
| /// Parses all common options and stores the result in the struct instance. | |||
| void prepare(CommandArgs args) | |||
| { | |||
| registry_urls = environment.get("DUB_REGISTRY", null).split(";"); | |||
There was a problem hiding this comment.
I think this should rather be done in the Dub class, so that the command line module ideally performs pure command line parsing (I realize that it already reads DFLAGS, which should really be refactored). A new mode for SkipPackageSuppliers could then also be added to enable choosing between using only those specified on the command line, or also those coming from the environment.
There was a problem hiding this comment.
It's weird to pass package suppliers to new Dub only to have it add more package suppliers internally.
Also the env variables are simple alternatives to the command line options, so treating them somewhere else doesn't seem too sensible.
There was a problem hiding this comment.
We could add env var support to CommandArgs or do sth. similar, if a more structured approach seems appropriate.
There was a problem hiding this comment.
It's weird to pass package suppliers to new Dub only to have it add more package suppliers internally.
But it will add more anyway, the default one and the ones from the config file. Also, Dub already reads DUBPATH for example. So just based on the current state, adding it there would cause a bit less additional confusion - but in general I'd say all of this should be controllable from outside of the Dub class and a more principled separation would of course be even better.
Adding a generic way to handle env vars as an alternative to certain command line arguments also open up the possibility for better auto-generated command line help, so that sounds like a good idea. The only thing that should be there, and this is kind of orthogonal to this PR, is a way to pass a custom environment, so that it's possible to opt-out when the package is used as a library. This actually applies to the whole library, though, and not just to the command line module.
There was a problem hiding this comment.
OK, makes sense to move to dub.d then. Abstracting out environment could indeed make sense, but having a library react to env vars has it's use-case as well (e.g. CURL).
- multiple URLs separated by semicolon
MartinNowak
force-pushed
the
env_registry_urls
branch
from
967436c
to
234a52c
Compare
|
Okay, I've opened two issues to remember this. |
