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

Exclude 3rdparty directories from license header generation #14165

Merged
merged 1 commit into from
Jan 6, 2023

Conversation

jlaur
Copy link
Contributor

@jlaur jlaur commented Jan 5, 2023

Related to #14154, see #14154 (comment)

Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
@jlaur jlaur added the infrastructure Build system and Karaf related issues and PRs label Jan 5, 2023
@jlaur jlaur requested a review from a team as a code owner January 5, 2023 20:42
Copy link
Member

@kaikreuzer kaikreuzer left a comment

Choose a reason for hiding this comment

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

Lgtm, although I think there should be no 3rd party code using our namespace (this should not be allowed), so this is maybe something to look at separately (I see that's the case for lametrictime).

@kaikreuzer kaikreuzer merged commit ff158e2 into openhab:main Jan 6, 2023
@kaikreuzer kaikreuzer added this to the 4.0 milestone Jan 6, 2023
@jlaur
Copy link
Contributor Author

jlaur commented Jan 6, 2023

@kaikreuzer - I can have a look at this also. I'm wondering though which namespace to use, since at some point this must have been changed to org.openhab. But even when fixed for the concrete case, there would still be a risk of messing with 3rd party license headers in case something new would be introduced by mistake again. So probably we should still consider exclusion a permanent solution for additional safety.

@kaikreuzer
Copy link
Member

I faintly remember that I was involved in adding this code to the lametric binding, so I can dig for the reasons why it is like it is.

we should still consider exclusion a permanent solution for additional safety.

Definitely yes! This PR is not considered a temporary solution.

@jlaur
Copy link
Contributor Author

jlaur commented Jan 6, 2023

@kaikreuzer - now looking into the last missing pieces of the puzzle, we also have this issue of two files with openHAB header living inside the 3rdparty directory. They were included in the search & replace, but won't be handled automatically by mvn license:format next time, so we need to decide what to do. One of them is lametrictime again. I didn't check yet, but if there are no other license headers, I guess we could move the files into the org.openhab namespace and have them handled normally.

diff --git a/bundles/org.openhab.binding.lametrictime/src/3rdparty/java/org/openhab/binding/lametrictime/api/common/impl/typeadapters/imported/CustomizedTypeAdapterFactory.java b/bundles/org.openhab.binding.lametrictime/src/3rdparty/java/org/openhab/binding/lametrictime/api/common/impl/typeadapters/imported/CustomizedTypeAdapterFactory.java
index 03091e1ab8..3508d2ad52 100644
--- a/bundles/org.openhab.binding.lametrictime/src/3rdparty/java/org/openhab/binding/lametrictime/api/common/impl/typeadapters/imported/CustomizedTypeAdapterFactory.java
+++ b/bundles/org.openhab.binding.lametrictime/src/3rdparty/java/org/openhab/binding/lametrictime/api/common/impl/typeadapters/imported/CustomizedTypeAdapterFactory.java
@@ -1,5 +1,5 @@
 /**
- * Copyright (c) 2010-2022 Contributors to the openHAB project
+ * Copyright (c) 2010-2023 Contributors to the openHAB project

diff --git a/bundles/org.openhab.binding.smsmodem/src/3rdparty/java/org/smslib/UnrecoverableSmslibException.java b/bundles/org.openhab.binding.smsmodem/src/3rdparty/java/org/smslib/UnrecoverableSmslibException.java
index 5ae22b8a62..e44b41162c 100644
--- a/bundles/org.openhab.binding.smsmodem/src/3rdparty/java/org/smslib/UnrecoverableSmslibException.java
+++ b/bundles/org.openhab.binding.smsmodem/src/3rdparty/java/org/smslib/UnrecoverableSmslibException.java
@@ -1,5 +1,5 @@
 /**
- * Copyright (c) 2010-2022 Contributors to the openHAB project
+ * Copyright (c) 2010-2023 Contributors to the openHAB project

Oh, and last thing: Now that we are already discussing license headers, can you have a look at #14134 (comment) and possible comment? 🙂

@jlaur jlaur deleted the license_headers_3rdparty branch January 6, 2023 19:58
borazslo pushed a commit to borazslo/openhab-mideaac-addon that referenced this pull request Jan 8, 2023
…14165)

Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
@jlaur
Copy link
Contributor Author

jlaur commented Feb 15, 2023

I faintly remember that I was involved in adding this code to the lametric binding, so I can dig for the reasons why it is like it is.

Did you find/remember the reason? 🙂

@kaikreuzer
Copy link
Member

Haha, I said I "can" check, I didn't say I would do. 😉
But thanks for the ping. I just had a look and found in the NOTICE file the reference to the original lib at https://github.com/syphr42/liblametrictime-java.
Afair, @syphr42 planned to maintain this library independently from openHAB, so that it can be used by other projects as well.
Since the maintenance of liblametrictime-java seems to have stopped, I actually wonder if @syphr42 would be willing to contribute this code under EPLv2, so that we could move it from the 3rdparty folder to the official source code of the binding. @syphr42 Wdyt, would that be ok for you?

@mhilbush
Copy link
Contributor

I also tried to contact @syphr42 a while back regarding the sleepiq binding, which also happens to have a 3rd party library that I modified pretty extensively in my PR here. But I got no response.

If it would be ok with @syphr42, I would be happy to move the sleepiq 3rdparty lib into the official openHAB source code for the sleepiq binding and make whatever mods are required to make it compliant. But in the absence of a response from @syphr42, what do we do?

@syphr42
Copy link

syphr42 commented Feb 15, 2023

@kaikreuzer @mhilbush I would be happy to contribute the code under EPLv2. I had a lot of ambitions for those bindings early on, but then I stepped away from openHAB for a bit and when I tried to come back wasn't able to get the development environment working for the latest incarnation of openHAB. Eventually it fell off my radar again. Happy to help get this in shape to make it easier to maintain and sorry for not responding sooner!

Let me know what you need from me.

@mhilbush
Copy link
Contributor

@syphr42 Thanks!!

@kaikreuzer @jlaur I would be happy to rework the sleepiq binding to roll the sleepiq api 3rd party library into the official source code of the binding, and make the changes necessary to make the code compliant (I don't think there's much to do). If that's ok with you, I will mark my sleepiq PR as WIP until I complete that. Ok?

@kaikreuzer
Copy link
Member

@syphr42 Awesome, thank you so much!
In case you ever want to get back into openHAB development, feel free to reach out - I'll be happy to support you in getting a dev env up and running!

@jlaur I'll prepare a PR to adapt the lametrictime binding, so that we no longer need the 3rdparty source folder in there.

@mhilbush
Copy link
Contributor

FYI I've updated my PR for sleepiq to migrate all the 3rd party code into the official binding code.

@kaikreuzer
Copy link
Member

Done for LametricTime: #14425

nemerdaud pushed a commit to nemerdaud/openhab-addons that referenced this pull request Feb 28, 2023
…14165)

Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
renescherer pushed a commit to renescherer/openhab-addons that referenced this pull request Mar 23, 2023
…14165)

Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infrastructure Build system and Karaf related issues and PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants