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

Cleanup mislabeled BSD license #37

Merged
merged 1 commit into from
Nov 6, 2020
Merged

Cleanup mislabeled BSD license #37

merged 1 commit into from
Nov 6, 2020

Conversation

tfoote
Copy link
Contributor

@tfoote tfoote commented Jan 31, 2020

This was propagated due to an invalid license in ament_lint which is proposed to be fixed at: ament/ament_lint#209

@tfoote tfoote requested a review from a team as a code owner January 31, 2020 23:37
Copy link
Member

@wjwwood wjwwood left a comment

Choose a reason for hiding this comment

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

It looks strange to me that these files are licensed BSD? I'm not sure why, maybe @dirk-thomas can comment, since they were added in #25?

// * Neither the name of the copyright holder nor the names of its
// contributors may be used to endorse or promote products derived from
// this software without specific prior written permission.

Copy link
Member

Choose a reason for hiding this comment

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

Not having // here looks wrong to me.

@dirk-thomas
Copy link
Member

maybe @dirk-thomas can comment, since they were added in #25?

#25 is only a redo of #4. And in these patches only the license of the visibility macros is BSD (see https://github.com/ros2/rcpputils/pull/25/files#diff-974160601a417a6b78e21e73844e59f3). I think that file can safely be changed to Apache. I haven't looked at the other cases.

@brawner brawner mentioned this pull request Feb 5, 2020
19 tasks
@emersonknapp
Copy link
Contributor

@tfoote is the expectation that the lint check here will pass when ament/ament_lint#209 is merged? In what order should things get merged?

@tfoote
Copy link
Contributor Author

tfoote commented Feb 8, 2020

This should pass with ament/ament_lint#205 it should land before ament/ament_lint#209 causes it to fail.

include/rcpputils/filesystem_helper.hpp Outdated Show resolved Hide resolved
include/rcpputils/split.hpp Outdated Show resolved Hide resolved
include/rcpputils/split.hpp Outdated Show resolved Hide resolved
include/rcpputils/split.hpp Outdated Show resolved Hide resolved
include/rcpputils/split.hpp Outdated Show resolved Hide resolved
include/rcpputils/visibility_control.hpp Outdated Show resolved Hide resolved
include/rcpputils/visibility_control.hpp Outdated Show resolved Hide resolved
include/rcpputils/visibility_control.hpp Outdated Show resolved Hide resolved
include/rcpputils/visibility_control.hpp Outdated Show resolved Hide resolved
@ahcorde
Copy link
Contributor

ahcorde commented Apr 17, 2020

@tfoote are the changes in the licenses still need it? I think the copyright test is passing in master, right ?

But the change in the package.xml is needed

@clalancette
Copy link
Contributor

@ahcorde This change is still needed to update the license from "BSD License 2.0" (which doesn't really exist) to the proper BSD 3-Clause. The license wording doesn't change, only the formatting and what we call it. Thus, I think this is still needed.

Another review pass would be appreciated. Thanks!

@clalancette
Copy link
Contributor

CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@clalancette clalancette merged commit 4ecb8d8 into master Nov 6, 2020
@clalancette clalancette deleted the bsd3clause_fixup branch November 6, 2020 15:31
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.

6 participants