Skip to content

Conversation

steveklabnik
Copy link
Contributor

Hi there, thanks for this great project! I am using the MPL in some places, and so would love built-in support for it. It's really nice that it was so easy to use a content block to get started, but I figured upstreaming this would be nice :)

I am not sure if I should be adding a test or anything, happy to make any modifications you'd like. I did verify that this works by running my branch against a sample codebase, you can see that result here: https://github.com/steveklabnik/ci-license-test/pull/4

This template follows Mozilla's official guidance, located at
https://www.mozilla.org/en-US/MPL/headers/
@wu-sheng wu-sheng added this to the 0.3.0 milestone Jan 6, 2022
@wu-sheng wu-sheng added the enhancement New feature or request label Jan 6, 2022
@wu-sheng wu-sheng requested a review from kezhenxu94 January 6, 2022 23:40
@wu-sheng
Copy link
Member

wu-sheng commented Jan 6, 2022

I have assigned the reviewer, but he has some personal stuff for 2 weeks. So this PR will be pending a little longer than usual.Please wait.

Comment on lines +1 to +3
This Source Code Form is subject to the terms of the Mozilla Public
License, v. 2.0. If a copy of the MPL was not distributed with this
file, You can obtain one at https://mozilla.org/MPL/2.0/.
Copy link
Member

Choose a reason for hiding this comment

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

I can confirm the content of this header matches the official version, https://www.mozilla.org/en-US/MPL/headers/

@MoGuGuai-hzr
Copy link
Contributor

Putting MPL-2.0 header template into assets/header-templates is a simplified solution which specifies the license header content in .licenserc.yaml. But remember, you still need to specify spdx-id of the license in .licenserc.yaml.

The current test has not considered other headers except apache-2.0. You can find the unit test TestCheckFile at in pkg/header/check_test.go. It uses a default ConfigHeader which will test apache-2.0 only.

In order to support other headers, maybe you should modify it and add some test data in test/testdata/**. In addition, the current test data is also only for apache-2.0, in order to deal with other headers, you may modify the directory structure of the test data.

@kezhenxu94
Copy link
Member

Thank you @steveklabnik for opening this PR and @MoGuGuai-hzr for reviewing!!!

The changes looks good to me. As for the tests, now we just want to test the mechanism that finding license header and comparing the header works, adding tests for each license might be boring and duplicating so I don't require (nor reject) it if anyone would like to add.

@kezhenxu94 kezhenxu94 merged commit ce2b365 into apache:main Jan 17, 2022
@steveklabnik steveklabnik deleted the add-mpl branch January 18, 2022 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants