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

Convert legacy NPM licenses to a format expected by composer. #88

Closed
wants to merge 2 commits into from

Conversation

richgerdes
Copy link

Convert older npm license format to one supported by composer. See #87.

} else {
$licenses[] = $packageLicense;
}
$release['license'] = $licenses;
Copy link
Member

Choose a reason for hiding this comment

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

License preparation should be refactored into prepareLicense function and used like this:

$release['license'] = $this->prepareLicense($package->getLicense());

Thanks!

Copy link
Member

@hiqsol hiqsol left a comment

Choose a reason for hiding this comment

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

Thank you.
I'll test it and then merge.
Do you know good candidates for testing?

@richgerdes
Copy link
Author

richgerdes commented Jun 19, 2018

I know tv4 and debuglog were the packages for which the issue presented itself, you could try starting with them

@hiqsol
Copy link
Member

hiqsol commented Jun 21, 2018

This change should be done in composer-asset-plugin
I've created PR fxpio/composer-asset-plugin#325
Please don't close this PR and your issue until the problem solved.

@hiqsol hiqsol self-assigned this Jun 21, 2018
@hiqsol hiqsol added the bug label Jun 21, 2018
@hiqsol
Copy link
Member

hiqsol commented Jul 5, 2018

The fix was merged in composer-asset-plugin.
I've updated production so everything should be ok now.

@hiqsol hiqsol closed this Jul 5, 2018
@rodrigoaguilera
Copy link

Thank you!

@richgerdes
Copy link
Author

Thanks for getting this moved into composer-asset-plugin!

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

Successfully merging this pull request may close these issues.

3 participants