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

range resolver don't output directly #4065

Merged

Conversation

@memsharded
Copy link
Member

@memsharded memsharded commented Dec 5, 2018

Changelog: Feature: Display the version-ranges resolutions in a cleaner way.
Docs: omit

Need it for graph-lock

@ghost ghost assigned memsharded Dec 5, 2018
Copy link
Member

@jgsogo jgsogo left a comment

Why not mocking the output? No modification needed, it looks like an output...

class GrabOutput:
    def __init__(self):
        self._output = []

    def info(self, msg):
        self._output.append((INFO, msg))

    ...

Also, I will refactor the RangeResolver, I don't like havint the output as a class member self._output:

class RangeResolver:
    def resolve(self, ..., output=NoneOutput()):
        ....
        output.success("....")

Note.- NoneOutput would be an empty implementation of an output class

conans/test/integration/version_ranges_diamond_test.py Outdated Show resolved Hide resolved
@memsharded
Copy link
Member Author

@memsharded memsharded commented Dec 5, 2018

Because I don't want the bare output. Actually, I want an structured information result of the version ranges resolution that have happened. Was going to use a dict of dicts, but preferred to keep it simple for the purpose, as first iteration. The output might become too noisy for users with heavy usage of version ranges, so some processing and compacting will be necessary.

jgsogo
jgsogo approved these changes Dec 11, 2018
Copy link
Member

@jgsogo jgsogo left a comment

memsharded want to push this forward for the graph-info one, I think we have to revisit the only-write-the-output-if-it-succeed thing, and maybe adopt this philosophy somewhere else (it is not useful to have some lines of output if the logic block is going to fail in the last step).

@lasote, have a look at it, and merge if you don't see anything important.

@lasote lasote merged commit fe58553 into conan-io:develop Dec 12, 2018
2 checks passed
@ghost ghost removed the stage: review label Dec 12, 2018
@memsharded memsharded deleted the feature/range_solver_capture_output branch Dec 12, 2018
@memsharded memsharded added this to the 1.11 milestone Dec 12, 2018
grisumbras pushed a commit to grisumbras/conan that referenced this issue Dec 27, 2018
* range resolver don't output directly

* fixing tests

* removed print
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants