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

Consider Composer dev dependencies #486

Merged
merged 3 commits into from
Mar 17, 2022

Conversation

digilist
Copy link
Contributor

I noticed that composer dev dependencies haven't been considered by this package, so I wanted to add this.

As I am not sure about the BC policy of this library, I made this a configuration option, so that people do not automatically get all the license updates. In general I would consider this enabled by default though, wdyt?

Copy link
Contributor

@jonabc jonabc left a comment

Choose a reason for hiding this comment

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

🚀 ✨ thanks for opening this PR! One small suggestion to keep ruby style consistency, and can you update the composer fixtures and add a test case that exercises the new config option? Let me know if any of how to make that work is unclear!

As I am not sure about the BC policy of this library, I made this a configuration option, so that people do not automatically get all the license updates. In general I would consider this enabled by default though, wdyt?

Dev or test dependencies do not normally ship with a product, so are not normally a primary concern where OSS compliance is involved. I think leaving the default to false and making this opt-in like the solution in this PR is the best strategy 👍

lib/licensed/sources/composer.rb Outdated Show resolved Hide resolved
@digilist
Copy link
Contributor Author

Thanks for the quick feedback! I guess I was just happy to get the code working and forgot about testing 😅

Copy link
Contributor

@jonabc jonabc left a comment

Choose a reason for hiding this comment

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

🚀 looks great, thanks! I'm putting out a release for licensed later today so this should be available for use relatively soon 👍

I'm going to ignore the license metadata verification failure, the issue is related to the running the workflow and not anything specific to the code changed here.

@jonabc jonabc merged commit c26b75f into github:master Mar 17, 2022
@digilist
Copy link
Contributor Author

Thank you very much! 👍

jonabc added a commit that referenced this pull request Mar 17, 2022
## 3.6.0

2022-03-17

### Added

- Composer dev dependencies can optionally be included in enumerated PHP dependencies (:tada: `@digilist` #486)
- Getting started usage documentation (#483)
- Initial support for NPM workspaces (#485)

### Changed

- Transitive dependencies are now enumerated by the `pip` source (#480)

### Fixed

- `licensed cache --force` will now correctly overwrite existing license classifications (#473)
@jonabc jonabc mentioned this pull request Mar 17, 2022
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.

2 participants