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

Update composer.json #112

Merged
merged 1 commit into from
Dec 27, 2017
Merged

Conversation

Dylan-DPC-zz
Copy link
Contributor

Laravel doesn't follow semver. So having ^5.x, 5.* or >=5.x will mean that composer will install a future version even though that version breaks the package.

@mikebronner mikebronner self-assigned this Dec 16, 2017
@mikebronner
Copy link
Member

Hi @Dylan-DPC, thanks for the PR! At this point I am leaning to not specify the Laravel minor version, knowing that Laravel does not respect semver. My initial thinking is that between using the cache and serviceprovider functionalities provided in the two dependencies, the chances of the functionality having breaking changes are small. And if they do occur it will be something that will be easily fixed.

On the other hand, if we do implement the minor version checks in composer.json, we will be bound to push un update every 6 months, and if we don't do it within a reasonable amount of time, people who do upgrade Laravel will not be able to use this package until we do.

Hence my thinking is that it will be less maintenance to not worry about versioning problems, until there actually is a problem. :) I'm keeping this open though, while I think on this over the next few days. :)

Thanks again!

Copy link
Member

@mikebronner mikebronner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See conversation.

@mikebronner
Copy link
Member

I've been mulling this over and am leaning toward accepting your PR. Give me a few more days, I'll work on it over the weekend. :) Thanks!

@mikebronner mikebronner merged commit d414c4e into geocoder-php:master Dec 27, 2017
@Dylan-DPC-zz Dylan-DPC-zz deleted the patch-1 branch December 27, 2017 17:34
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.

3 participants