Skip to content

Add null check when getting fragment host wire provider#1860

Merged
HannesWell merged 1 commit intoeclipse-pde:masterfrom
Kummallinen:issue-1819
Jul 9, 2025
Merged

Add null check when getting fragment host wire provider#1860
HannesWell merged 1 commit intoeclipse-pde:masterfrom
Kummallinen:issue-1819

Conversation

@Kummallinen
Copy link
Copy Markdown
Contributor

If the host is missing or does not build an NPE occurred (issue #1819)

@Kummallinen
Copy link
Copy Markdown
Contributor Author

@HannesWell this change resolves the NPE issue for me but I don't know what impacts this has on what that code was intended to achieve

Comment thread ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/DependencyManager.java Outdated
Copy link
Copy Markdown
Contributor

@laeubi laeubi left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@laeubi laeubi marked this pull request as ready for review July 9, 2025 12:34
Copy link
Copy Markdown
Contributor

@merks merks left a comment

Choose a reason for hiding this comment

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

This looks good to me. Thanks for contributing.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jul 9, 2025

Test Results

   765 files     765 suites   48m 22s ⏱️
 3 611 tests  3 557 ✅  54 💤 0 ❌
10 833 runs  10 670 ✅ 163 💤 0 ❌

Results for commit 9f0efd1.

♻️ This comment has been updated with latest results.

Copy link
Copy Markdown
Member

@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.

Thank you for your contribution, that's great.

I just wonder about the cause of the NPE and if it could be fixed at its root?
Do you have a 'working' (or more not working) example of the referenced issue and could check why the wiring is absent?

Comment thread ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/DependencyManager.java Outdated
@Kummallinen
Copy link
Copy Markdown
Contributor Author

Thank you for your contribution, that's great.

I just wonder about the cause of the NPE and if it could be fixed at its root? Do you have a 'working' (or more not working) example of the referenced issue and could check why the wiring is absent?

For me the root cause was a fragment in my workspace where the host project didn't build due to dependency errors. Which seems a legitimate reason for the provider wiring to be missing

@HannesWell
Copy link
Copy Markdown
Member

I just wonder about the cause of the NPE and if it could be fixed at its root? Do you have a 'working' (or more not working) example of the referenced issue and could check why the wiring is absent?

For me the root cause was a fragment in my workspace where the host project didn't build due to dependency errors. Which seems a legitimate reason for the provider wiring to be missing

Yes, makes sense. Then please remove logging the error and then this change is ready for submission.

If the host is missing or does not build an NPE occurred (issue eclipse-pde#1819)
@Kummallinen
Copy link
Copy Markdown
Contributor Author

root cause was a fragment in my workspace where the host project didn't build due to dependency errors. Which seems a legitimate reason for the provider wiring to be missin

Done. Thank you for the quick review

Copy link
Copy Markdown
Member

@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.

Looks good, thank you!
Nice contribution, this is very welcome.

@HannesWell HannesWell merged commit c82e614 into eclipse-pde:master Jul 9, 2025
19 checks passed
@HannesWell HannesWell linked an issue Jul 9, 2025 that may be closed by this pull request
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.

SWTBot plug-ins the workspace give error in API Analysis Builder

4 participants