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

Add slf4j configuration for php, committers and scout #47

Merged
merged 1 commit into from
Aug 25, 2023

Conversation

jonahgraham
Copy link
Contributor

Because the slf4j.simple is not included "naturally" by any of the features already in php, committers or scout, include it explicitly here.

Includes revert "Don't add slf4j.simple to products that don't include slf4j already" This reverts commit a196924.

Fixes #27

Because the slf4j.simple is not included "naturally"
by any of the features already in php, committers or
scout, include it explicitly here.

Includes revert "Don't add slf4j.simple to products that don't include slf4j already"
This reverts commit a196924.

Fixes eclipse-packaging#27
@jonahgraham
Copy link
Contributor Author

@HannesWell this seems one way to resolve the problem I mentioned in #43 (comment) (part 1)

Copy link
Contributor

@HannesWell HannesWell left a comment

Choose a reason for hiding this comment

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

@HannesWell this seems one way to resolve the problem I mentioned in #43 (comment) (part 1)

Yes this looks good.
Actually I expected that slf4j.api/simple will be pulled in from the eclipse/updates repo automatically.
Does the product contain any bundle that requires SLF4J?

@jonahgraham
Copy link
Contributor Author

Actually I expected that slf4j.api/simple will be pulled in from the eclipse/updates repo automatically.

AFAICT the bundles that worked (like cpp) pulled in slf4j.api/simple, but the others pulled in slf4j.api/nop without this change. I don't know how to track down the why though, so perhaps all packages (without logback) should have this slf4j.api/simple added to the product file?

@jonahgraham jonahgraham merged commit b76936f into eclipse-packaging:master Aug 25, 2023
1 check passed
@jonahgraham jonahgraham deleted the slf4j branch August 25, 2023 01:25
@HannesWell
Copy link
Contributor

Actually I expected that slf4j.api/simple will be pulled in from the eclipse/updates repo automatically.

AFAICT the bundles that worked (like cpp) pulled in slf4j.api/simple, but the others pulled in slf4j.api/nop without this change. I don't know how to track down the why though, so perhaps all packages (without logback) should have this slf4j.api/simple added to the product file?

If slf4j.nop is available in the SimRel repo then it is probably random if simple or nop is included, unless some feature also includes slf4j.simple.
So yes it would probably be a good idea to add simple explicitly where we expect/want it.
Should I provide a PR? slf4j.simple is probably enough. It will pull in the slf4j.api anyways.

@merks
Copy link
Contributor

merks commented Aug 25, 2023

Nothing with SimRel itself strictly requires what's provided by either one:

image

This thing is happy with different possibilities:

image

@jonahgraham
Copy link
Contributor Author

So yes it would probably be a good idea to add simple explicitly where we expect/want it.

I will create such a PR, thanks for offering though. It woul d be great if you could review it at some point though.

jonahgraham added a commit to jonahgraham/packages that referenced this pull request Aug 25, 2023
jonahgraham added a commit to jonahgraham/packages that referenced this pull request Aug 25, 2023
jonahgraham added a commit that referenced this pull request Aug 25, 2023
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.

Update EPP for SLF4J v2 changes coming from platform
3 participants