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

Changed package requirements to include distro version 1.5.0 #7461

Merged
merged 1 commit into from Aug 24, 2020
Merged

Changed package requirements to include distro version 1.5.0 #7461

merged 1 commit into from Aug 24, 2020

Conversation

DominikDeak
Copy link
Contributor

@DominikDeak DominikDeak commented Jul 31, 2020

Changelog: Fix: Changed requirements.txt to include distro package version 1.5.0.
Docs: Omit

Close #7455

  • Refer to the issue that supports this Pull Request.
  • If the issue has missing info, explain the purpose/use case/pain/need that covers this Pull Request.
  • I've read the Contributing guide.
  • I've followed the PEP8 style guides for Python code.
  • I've opened another PR in the Conan docs repo to the develop branch, documenting this one.

Note: By default this PR will skip the slower tests and will use a limited set of python versions. Check here how to increase the testing level by writing some tags in the current PR body text.

@CLAassistant
Copy link

CLAassistant commented Jul 31, 2020

CLA assistant check
All committers have signed the CLA.

@uilianries
Copy link
Member

Hi @DominikDeak ! Thanks for your contribution! Could you please sign the CLA.

@memsharded memsharded added this to the 1.29 milestone Aug 3, 2020
@jgsogo
Copy link
Contributor

jgsogo commented Aug 5, 2020

Hi! We need a policy for this kind of external dependencies. Changing the version can change the output of some Conan autodetect features. This is very related to the compiler detection @SSE4 implemented, almost same situation.

I agree to update this library, but maybe we should add a disclaimer to the docs associated with the affected functions to tell the users that we are using distro under the hood and, depending on the version used, the results can be different. If this is not ok, then we shouldn't add a range for these dependencies, but a fixed version.

We need to be aligned, wdyt @conan-io/barbarians ?

@uilianries
Copy link
Member

depending on the version used, the results can be different

@jgsogo why not improving our regression tests instead? So we can predict any change from external packages.

@jgsogo
Copy link
Contributor

jgsogo commented Aug 5, 2020

depending on the version used, the results can be different

@jgsogo why not improving our regression tests instead? So we can predict any change from external packages.

Same problem as with the compiler detection, how many different docker images we need to run? How many different OS x platforms x ...? And those docker images... are they available or do we need to maintain them ourselves?

@memsharded
Copy link
Member

I think we should work with this ideas in mind:

  • Try to minimize the external dependencies and surface area. Embed when possible. Delegate to the user space when possible. Fork under different name when it makes sense. In general be more aware when introducing dependencies.

  • Testing is indeed difficult. We will never cover all the cases with enough confidence, as the space is huge.

  • Agree that it is necessary to document it correctly in the docs. I lean to accepting the range and documenting than being strict on the range and having users complaining because of conflicting distro versions.

@jgsogo
Copy link
Contributor

jgsogo commented Aug 13, 2020

I'm adding a warning to the docs here (almost a disclaimer)

@memsharded memsharded merged commit 47928cb into conan-io:develop Aug 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.

Enable support for 'distro 1.5.0'
6 participants