Skip to content

[wip] [5 of #107] add dynamic options #125

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

Merged
merged 2 commits into from
Dec 20, 2020

Conversation

bollwyvl
Copy link
Contributor

This encapsulates the state that does change, as highlighted in #124, in a single, relatively lenient object, and changes most function signatures to accept inputs, options or both.

@bollwyvl: I haven't tested this one out. 🤞

On the next PR

refactor the installer provider

One of the first "irregularities" is that runInstaller is allowed to calculate its own outputPath, which requires access to mutable state. If we split up that behavior, we can get a lot of reuse, where it will be more straightforward to add additional "well-known" installer providers.

The idea here is to break up the installer provisioning step even more. There should be a significant net negative line count in setup.ts, and encapsulate the logic more succinctly along with its effects.

This is where we want to leave enough breathing room to handle micromamba #75 without actually building it right now.

@goanpeca
Copy link
Member

LGTM @bollwyvl gave it a quick look :)

@bollwyvl bollwyvl marked this pull request as ready for review December 20, 2020 23:15
@bollwyvl
Copy link
Contributor Author

Yeah, had another look I don't see anything else glaring at this step.

@goanpeca goanpeca merged commit 9ed7b04 into conda-incubator:master Dec 20, 2020
@goanpeca goanpeca added this to the v2.1.0 milestone Dec 24, 2020
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

Successfully merging this pull request may close these issues.

None yet

2 participants