Use getPrettyName; cater for legacy incorrect package names. #50
Conversation
|
Restarted build |
|
Failed again so github.com issues can be ruled out I think and it's some issue with the |
|
@gitlost Yes, but not in the way you think:
|
|
Ha excellent! |
|
Hmm, can't think of a way around it, which is sad as it's a key test for this PR. What would you think of adding a if ( getenv( 'TRAVIS' ) ) {
$skip_tags[] = '@local-only';
} |
|
Ah it would fail randomly locally too - will just remove it... |
I'm not opposed and don't have a better suggestion, if you think this is the best path forward. However, I'm not sure of the utility of an integration test that never runs in the CI environment. |
|
Requested a review from @schlessera because I'd like his eyes on it specifically. |
| */ | ||
| private function check_git_package_name( $package_name ) { | ||
| // Generate raw git URL of composer.json file. | ||
| $raw_content_url = 'https://raw.githubusercontent.com/' . $package_name . '/master/composer.json'; |
There was a problem hiding this comment.
I think we still have a problem here.
The branch master is hardcoded, so the code will pull in the wrong composer.json file if a different branch was requested.
Not a bug with this PR, but rather a bug we already had in the previous iteration.
There was a problem hiding this comment.
Ah right so check_git_package_name() needs to take a $version arg as well - should this be a new issue/PR or deal with it here?
There was a problem hiding this comment.
I'd say a new PR, as the bug is already in there. This way, we can concentrate on the issue at hand first, and deal with the other bug separately.
There was a problem hiding this comment.
I'll merge this one and create a new issue for that bug.
Use getPrettyName; cater for legacy incorrect package names.
Closes #43
Partially reverts #49
(TLDR: Alas #49 had a number of issues. This PR tries to deal with them and also cater for legacy incorrect names in
composer.json.)Uses
Package::getPrettyName()instead ofPackage::getName()throughoutPackage_Command(except where needed for legacy situations), asgetPrettyName()is the actual case-sensitive name that appears incomposer.json.Allows and corrects for legacy incorrect names being in
composer.json, which cause strange behaviour if present. A warning is printed on detection.Fixes a case-sensitivity bug in
Composer\Json\JsonManipulatorre package names, which causes mixed-case package names inrequireandrepositoriesto be left incomposer.json, by adding acaseInsensitivearg toaddLink(),addSubNode()andremoveSubNode(). This required a full copy ofComposer\Json\JsonManipulator(due to its use of private member variables) with adaptations.Reverts the addition in #49 of an outwardly visible
pretty_name, as having two names only confuses things and there really can't be BC issues as always outputting lowercase name is wrong. For BC withpackage-command1.0.8, still setspretty_nameso it won't fail if specified.Moves the mismatch code to its own function so it can be called to deal with the shortened form of git url also. Adds some error handling and expands the mismatch message with explicit before and after names.
Also copies the phpunit test for
JsonManipulatorfrom Composer, with some additions.