Skip to content
This repository has been archived by the owner on Apr 6, 2022. It is now read-only.

Use latest dns2 library from Packagist #70

Merged
merged 1 commit into from
Dec 29, 2015

Conversation

IlyaPokamestov
Copy link
Contributor

#51 Use latest stable pear/net_dns2 package from packagist instead Pear.

@cpliakas
Copy link
Contributor

cpliakas commented Sep 8, 2015

@DarioSwain Thanks for the contribution.

Don't worry about the scrutinizer stuff, the settings are a little too restrictive and need to be tweaked. Regarding the code change, I am 100% in favor of using the packagist version of this package. The reason why it is pinned to 1.3.2 is because it is (or was) the same version that is used on the Acquia Cloud platform, and if possible I want to maintain parity.

I do need to check whether this is still the case, but in the meantime are there any specific reasons why we should upgrade to the 1.4 version, e.g. security issues, bug fixes, etc? I do think that keeping current is always the best option, I just want to understand the context so that we don't accidentally introduce instability if we don't have to.

Thanks again for the pull request.
Chris

@IlyaPokamestov
Copy link
Contributor Author

Hi @cpliakas,
Sorry for long reply.

I think it's a good point to use ~ for patch versions, you can see more information here: semver.
At now i don't see v1.3.2 tag for net_dns2 on GitHub, maybe make sense create issue for moving all version tags from Pear to GitHub. I check it later.
But now, i compare v1.3.2 and v1.4.1 and seems like it fix some bugs and provide more functionality, include normal autoloading. In you case, you only use Resolver component and i don't see any changes for it.

But I think you are right, you must check this version before merge this PR. From my side i don't see any reasons to use old version of packages, if you check pear, v1.4.1 mark as a stable. Maybe it good point to move all Acquia infrastructure to new version ... it only your choice

@IlyaPokamestov
Copy link
Contributor Author

@cpliakas please check this issue.
I update this PR, at now I use 1.3.2 version of net_dns2 package.
It more useful?

cpliakas added a commit that referenced this pull request Dec 29, 2015
Use latest dns2 library from Packagist
@cpliakas cpliakas merged commit 351b778 into acquia:master Dec 29, 2015
@cpliakas
Copy link
Contributor

@DarioSwain, sorry for the long turnaround on this. Thanks for the contribution!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants