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

Move VersionParser::formatVersion() to BasePackage::getFullPrettyVersion() #4086

Merged
merged 3 commits into from Jul 3, 2015

Conversation

legoktm
Copy link
Contributor

@legoktm legoktm commented May 31, 2015

Working towards #3545.

formatVersion() does not belong in VersionParser since it depends upon a
Package object, and is creating a more complete pretty formatted
version, not parsing anything.

The new getFullPrettyVersion() method can be seen as an extension to
getPrettyVersion(), and is located in BasePackage as a result.

I'm also not very good at naming things, and getFullPrettyVersion was the best I could come up with. Better suggestions welcome :)

}

return $package->getPrettyVersion() . ' ' . $package->getSourceReference();
return $package->getFullPrettyVersion($truncate);
Copy link
Member

Choose a reason for hiding this comment

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

It's ok for me to keep this around but I think it should do a trigger_error('VersionParser::formatVersion is deprecated, you should use $package->getFullPrettyVersion() instead', E_USER_DEPRECATED);

Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed. Along with a @deprecated annotation. That way you could easily catch it in Composer 2.0.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done both.

return $package->getPrettyVersion() . ' ' . $package->getSourceReference();
trigger_error(__METHOD__.' is deprecated. Use '.
'\Composer\Package\PackageInterface::getFullPrettyVersion() instead', E_USER_DEPRECATED);
return $package->getFullPrettyVersion($truncate);
Copy link
Member

Choose a reason for hiding this comment

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

Empty line before return construct.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

…ion()

Working towards composer#3545.

formatVersion() does not belong in VersionParser since it depends upon a
Package object, and is creating a more complete pretty formatted
version, not parsing anything.

The new getFullPrettyVersion() method can be seen as an extension to
getPrettyVersion(), and is located in BasePackage as a result.

Callers to VersionParser::formatVersion() were not updated in this
commit to demonstrate that no functionality was changed in this
refactor. They will be updated in a follow up commit.
Tests were moved to BasePackageTest.
@legoktm
Copy link
Contributor Author

legoktm commented Jul 2, 2015

Fixed code style, rebased against master, and then fixed failing tests due to a missing namespace import.

Seldaek added a commit that referenced this pull request Jul 3, 2015
Move VersionParser::formatVersion() to BasePackage::getFullPrettyVersion()
@Seldaek Seldaek merged commit 2438105 into composer:master Jul 3, 2015
@Seldaek
Copy link
Member

Seldaek commented Jul 3, 2015

Thanks :)

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.

None yet

4 participants