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

Upgrade nikolaposa/version to 4.0.0 #179

Closed
wants to merge 6 commits into from

Conversation

nikolaposa
Copy link
Contributor

@nikolaposa nikolaposa commented Dec 14, 2019

Proposed changes

Synchronize with changes made in Version 4.0.0 release.

  • fix Version and VersionCollection usage in accordance with their new API
  • fix assumption that input Version string prefix is not preserved when casting to string

@nikolaposa
Copy link
Contributor Author

nikolaposa commented Dec 14, 2019

This is just a trial run, I'll update require constraint as soon as nikolaposa/version is stable.

And when I already revealed that the new release of is near, are there any other things I could address in it?

@Ocramius
Copy link
Member

Patch seems good so far, except for a failure: https://travis-ci.org/Roave/BackwardCompatibilityCheck/jobs/624970650#L311

As for required additions in the upstream library, there's still this bug left: #37

In practice, we need to get both the version (normalized) and the original name of the symbol that led to that version. For example, 'release-1.2.3' or 'v1.2.3' or '1.2.3' are all equivalent, but we still want to know that a specific Version came from the string 'release-1.2.3'.

This allows us to check out symbols by their original name.

@nikolaposa
Copy link
Contributor Author

nikolaposa commented Dec 16, 2019

Ok, I'll take a look at that failure.

As for v1.2.3 issue, I always refer to: https://semver.org/#is-v123-a-semantic-version in conjunction with my library that tends to adhere to that semver document. So one way to solve this problem, while still keeping Version class within its original boundaries - object representing SemVer-compliant version number (v1.2.3 is not), is introducing additional, separate concept in it, something like:

class Tag
{
    private string $prefix;
    private Version $version;
}

I will consider some other solutions as well.

@nikolaposa
Copy link
Contributor Author

Verify BC Breaks stage is failing due to changes in fromRepository() method signature, caused by VersionsCollection -> VersionCollection rename.

@nikolaposa
Copy link
Contributor Author

For fixing #37, I can consider back-porting preserving of a Version string prefix in 3.2.x of my library.

@Ocramius
Copy link
Member

@nikolaposa no need - can happen in the next release

@nikolaposa nikolaposa changed the title [draft] Upgrade nikolaposa/version to 4.0.0 Upgrade nikolaposa/version to 4.0.0 Dec 30, 2019
Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

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

@nikolaposa if this passes as-is for you locally, we can merge once composer.json changes here.

composer.json Outdated Show resolved Hide resolved
Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

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

LGTM: will merge manually after this round of dependency upgrades is through :-)

@nikolaposa
Copy link
Contributor Author

Sounds good!

@Ocramius
Copy link
Member

Ocramius commented Feb 8, 2020

Moved to #199

@Ocramius Ocramius closed this Feb 8, 2020
Ocramius added a commit that referenced this pull request Feb 8, 2020
…on-dependency-to-4.0.0

#179 bump `nikolaposa/version` dependency to `4.0.0`
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.

2 participants