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

[twitter] rename binding to X #15809

Merged
merged 18 commits into from
Nov 15, 2023
Merged

[twitter] rename binding to X #15809

merged 18 commits into from
Nov 15, 2023

Conversation

lsiepel
Copy link
Contributor

@lsiepel lsiepel commented Oct 26, 2023

I guess X is here to stay for a while. This PR :

=> Renaming files, classes and documentation from Twitter to X.
=> Update twitter4j from 4.0.7 to 4.1.2

Changelogs:
https://twitter4j.org/2022/10/12/120
https://twitter4j.org/2022/10/21/264
https://twitter4j.org/2022/10/27/269

@computergeek1507 can you test this version?

Fixes #15370

Signed-off-by: Leo Siepel <[email protected]>
Signed-off-by: Leo Siepel <[email protected]>
@lsiepel lsiepel added the enhancement An enhancement or new feature for an existing add-on label Oct 26, 2023
@lsiepel lsiepel requested review from computergeek1507 and a team as code owners October 26, 2023 14:24
Signed-off-by: Leo Siepel <[email protected]>
@lsiepel
Copy link
Contributor Author

lsiepel commented Oct 26, 2023

Don't understand why the build is failing, might need some help.

@wborn
Copy link
Member

wborn commented Oct 26, 2023

I think you forgot to update the BOM:

<artifactId>org.openhab.binding.twitter</artifactId>

Signed-off-by: Leo Siepel <[email protected]>
Signed-off-by: Leo Siepel <[email protected]>
@lsiepel
Copy link
Contributor Author

lsiepel commented Oct 27, 2023

What can we do with the non-default translations files ? Is crowdin able to rename the files?

Signed-off-by: Leo Siepel <[email protected]>
Signed-off-by: Leo Siepel <[email protected]>
Signed-off-by: Leo Siepel <[email protected]>
Signed-off-by: Leo Siepel <[email protected]>
@lsiepel
Copy link
Contributor Author

lsiepel commented Oct 28, 2023

Need some help with the build error.

@wborn
Copy link
Member

wborn commented Nov 11, 2023

Message: Unable to resolve root: missing requirement [root] osgi.identity; osgi.identity=openhab-binding-x; type=karaf.feature; version=4.1.0.SNAPSHOT; filter:="(&(osgi.identity=openhab-binding-x)(type=karaf.feature)(version>=4.1.0.SNAPSHOT))" [caused by: Unable to resolve openhab-binding-x/4.1.0.SNAPSHOT: missing requirement [openhab-binding-x/4.1.0.SNAPSHOT] osgi.identity; osgi.identity=org.openhab.binding.x; type=osgi.bundle; version="[4.1.0.202310280753,4.1.0.202310280753]"; resolution:=mandatory [caused by: Unable to resolve org.openhab.binding.x/4.1.0.202310280753: missing requirement [org.openhab.binding.x/4.1.0.202310280753] osgi.wiring.package; filter:="(osgi.wiring.package=org.slf4j.impl)"]]

It's most likely caused by the dependency upgrade of twitter4j-core. Bnd does bytecode analysis on the embedded classes to see which classes are imported. If the packages of the classes are not provided by the bundle itself, Bnd adds package imports to the generated bundle manifest. The Karaf feature validation checks if all package imports in the feature bundle manifests can be satisfied by the installed bundles.

So in this case there is no bundle providing org.slf4j.impl. This is due to bad design of twitter4j-core because it shouldn't depend on internal implementation classes as they have no stable APIs and the implementation of these classes can change between any release. We use Pax Logging which does provide the public (stable) SLF4J APIs but for these reasons not the internal (unstable) implementation classes.

You can probably workaround this by adding !org.slf4j.impl.* to the bnd.importpackage property in the POM but it could be that this breaks the twitter4j-core logging.

Signed-off-by: Leo Siepel <[email protected]>
Signed-off-by: Leo Siepel <[email protected]>
Copy link
Member

@wborn wborn 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 addressing the comments! I have added a few more below:

Signed-off-by: Leo Siepel <[email protected]>
Signed-off-by: Leo Siepel <[email protected]>
@lsiepel
Copy link
Contributor Author

lsiepel commented Nov 12, 2023

Can we also do some smart renaming for the crowdin properties/files? Hope someone else picks up that task as i'm not familiair with crowdin.

Signed-off-by: Leo Siepel <[email protected]>
Signed-off-by: Leo Siepel <[email protected]>
Copy link
Member

@wborn wborn left a comment

Choose a reason for hiding this comment

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

Thanks! 👍

@wborn wborn merged commit 12f0212 into openhab:main Nov 15, 2023
2 checks passed
@wborn wborn added this to the 4.1 milestone Nov 15, 2023
@lsiepel lsiepel deleted the twitter-x branch November 15, 2023 09:35
@wborn
Copy link
Member

wborn commented Nov 22, 2023

Can you also create a PR to add some info for these changes in update.lst in the distro repo @lsiepel ?

andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Nov 26, 2023
austvik pushed a commit to austvik/openhab-addons that referenced this pull request Mar 27, 2024
Signed-off-by: Leo Siepel <[email protected]>
Signed-off-by: Jørgen Austvik <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature for an existing add-on (potentially) not backward compatible
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[twitter] rename to [x]
2 participants