Skip to content

Upgrade nikolaposa/version to 4.0.0 - #179

Closed
nikolaposa wants to merge 6 commits into
Roave:masterfrom
nikolaposa:NP/version4-migration
Closed

Upgrade nikolaposa/version to 4.0.0#179
nikolaposa wants to merge 6 commits into
Roave:masterfrom
nikolaposa:NP/version4-migration

Conversation

@nikolaposa

@nikolaposa nikolaposa commented Dec 14, 2019

Copy link
Copy Markdown
Contributor

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

nikolaposa commented Dec 14, 2019

Copy link
Copy Markdown
Contributor Author

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
Copy Markdown
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

nikolaposa commented Dec 16, 2019

Copy link
Copy Markdown
Contributor Author

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.

Comment thread src/Git/PickLastMinorVersionFromCollection.php
@nikolaposa

Copy link
Copy Markdown
Contributor Author

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

@nikolaposa

Copy link
Copy Markdown
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
Copy Markdown
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

@Ocramius Ocramius left a comment

Copy link
Copy Markdown
Member

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.

Comment thread test/unit/Git/GetVersionCollectionFromGitRepositoryTest.php
Comment thread composer.json Outdated

@Ocramius Ocramius left a comment

Copy link
Copy Markdown
Member

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
Copy Markdown
Contributor Author

Sounds good!

@Ocramius

Ocramius commented Feb 8, 2020

Copy link
Copy Markdown
Member

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