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

Requirements merge is broken #168

Closed
JoshKarpel opened this issue Jul 24, 2019 · 0 comments · Fixed by #171
Closed

Requirements merge is broken #168

JoshKarpel opened this issue Jul 24, 2019 · 0 comments · Fixed by #171
Assignees
Labels
bug Something isn't working

Comments

@JoshKarpel
Copy link
Contributor

JoshKarpel commented Jul 24, 2019

Because of this kind of logic:

base_requirements = base.pop('requirements', None)
settings_requirements = from_settings.pop('requirements', None)
if base_requirements is not None and settings_requirements is not None:
    core['requirements'] = f'({base_requirements}) && ({settings_requirements})'

which appears in two places in options.py, extra requirements are only merged in if there are also other requirements coming from some other place. This is obviously broken, because sometimes the other set of requirements won't exist. This needs better logic.

@JoshKarpel JoshKarpel added the bug Something isn't working label Jul 24, 2019
@JoshKarpel JoshKarpel self-assigned this Jul 24, 2019
JoshKarpel added a commit that referenced this issue Jul 24, 2019
JoshKarpel added a commit that referenced this issue Jul 26, 2019
@JoshKarpel JoshKarpel mentioned this issue Jul 26, 2019
JoshKarpel added a commit that referenced this issue Jul 29, 2019
* resolve #168 
* resolve #169
* resolve #165
* resolve #173
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant