Skip to content

Prefer caret constraints over wildcards - #22

Merged
Caitlin Russell (caitlinrussell) merged 3 commits into
microsoftgraph:masterfrom
curry684:patch-1
Jul 11, 2017
Merged

Prefer caret constraints over wildcards#22
Caitlin Russell (caitlinrussell) merged 3 commits into
microsoftgraph:masterfrom
curry684:patch-1

Conversation

@curry684

Copy link
Copy Markdown
Contributor

Specifically in this case this library was stopping Guzzle 6.3 from being installed while Semver guarantees its compatibility. You should always use caret constraints unless there is a very good reason not to.

The other changes are simply layout.

Specifically in this case this library was stopping Guzzle 6.3 from being installed while Semver guarantees its compatibility. You should always use caret constraints unless there is a very good reason not to.
@msftclas

This seems like a small (but important) contribution, so no Contribution License Agreement is required at this point. We will now review your pull request.
Thanks,
Microsoft Pull Request Bot

You should *never* specify the version explicitly. Packagist will infer it from git tags. Refer to https://getcomposer.org/doc/02-libraries.md#library-versioning
@caitlinrussell

Copy link
Copy Markdown

Unfortunately, Microsoft's open source policy requires us to register and specify a specific major and minor version of each package that we include in our projects.
I will register this component internally and update the version number to reflect this.

@curry684

Copy link
Copy Markdown
Contributor Author

Well, in that case the policy needs to change because it's actively breaking interoperability in the Packagist ecosphere, and it'll be rendering your library useless.

The whole point of Semver, which you are implicitly complying to by distributing via Composer and Packagist, is that minor/patch level updates are 'guaranteed' to be safe by the developer. Hence limiting to 6.2.* is overly limiting compared to ^6.2 (which translates to >=6.2 <7 internally) as, especially for a popular library like Guzzle, an application is likely to register multiple projects depending on it. This would obviously cause unsolveable issues if they depended on 6.1.*, 6.2.* and 6.3.* respectively.

Hence the correct solution is to use the caret and the minimum compatible version, likely for this library ^6.0 would also work, and let Composer figure it out (you can even unit test it automatically using Composer's --prefer-lowest flag). The minimum compatible version should only ever be raised if known compatibility issues arise and/or newly introduced features from minor versions are used. Dependency resolution is about keeping constraints as wide as possible to allow for the biggest compatibility between packages.

And on a completely different note: you should avoid hard depending on Guzzle whatsoever, and use HTTPlug instead. It's an abstraction layer allowing the application developer to choose the HTTP client of their liking, including even Guzzle 5, without affecting your library.

@caitlinrussell

Copy link
Copy Markdown

I completely agree. I am currently working with legal to see if there is any way we can loosen restrictions in this area. If we cannot, I will need to write a script to automatically submit new versions of dependent packages and update the version numbers once approved. I am hopeful that we will be able to use your solution instead.
I also agree that Guzzle is a bit too tightly coupled at the moment. There is work I'd like to do to abstract this to the point that developers can substitute their own HTTP client as you mentioned, which I think would greatly increase the audience that can use this library. I haven't looked into HTTPlug much but will investigate. However, I still want to allow users to use a default, configured HTTP client without having to do any work on their end to get up-and-running with Graph.

I am going to leave this PR open in the event that advice changes on being able to allow developers to use a range of dependencies.

@curry684

Copy link
Copy Markdown
Contributor Author

Thank you for being honest and open. Having a public thread like this with a Microsoft employee would have been unthinkable 10 years ago. I really appreciate the full turn your company has made since the days of Ballmer, and the way it is improving not only your products but also ours.

@msftclas

Niels Keurentjes (@curry684), thanks for signing the contribution license agreement. We will now validate the agreement and then the pull request.

Thanks, Microsoft Pull Request Bot

@curry684

Copy link
Copy Markdown
Contributor Author

Resolved conflicts.

@caitlinrussell

Copy link
Copy Markdown

Great news, we got approval to use caret constraints! This update will be released shortly.

@caitlinrussell
Caitlin Russell (caitlinrussell) merged commit c9a3b40 into microsoftgraph:master Jul 11, 2017
@curry684

Copy link
Copy Markdown
Contributor Author

Cheers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants