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

Check getBundle returns non-null after PDE Manifest-editor opened. #1377

Conversation

peddaiahjuturu
Copy link
Contributor

@peddaiahjuturu peddaiahjuturu commented Aug 14, 2024

Bug details found here #1374

(Regression introduced by #949 and reported in #864)
After opening MANIFEST.MF file in PDE Manifest-editor and delete the project, ll get NPE.

Fixes #1374

@peddaiahjuturu
Copy link
Contributor Author

Now, legal agreement check passed, @HannesWell , @laeubi , @alshamams, please do review this PR.

Copy link

github-actions bot commented Aug 14, 2024

Test Results

   285 files  ±0     285 suites  ±0   49m 55s ⏱️ - 1m 31s
 3 580 tests ±0   3 504 ✅ ±0   76 💤 ±0  0 ❌ ±0 
10 932 runs  ±0  10 701 ✅ ±0  231 💤 ±0  0 ❌ ±0 

Results for commit 70bc75c. ± Comparison against base commit 9b87c23.

♻️ This comment has been updated with latest results.

@HannesWell
Copy link
Member

HannesWell commented Aug 14, 2024

Thank you @peddaiahjuturu, the change looks good.

But please squash your second commit into your first so that we have a single commit for this PR.
You can use the message from your first commit and add the following line as body:

Fixes https://github.com/eclipse-pde/eclipse.pde/issues/1374

Then you can just force-push to this branch (no need to create a new PR).

@HannesWell HannesWell force-pushed the bugs/1374_NPE_after_opening_MANIFEST_file_and_project_deletion branch from 46db1ca to 70bc75c Compare August 15, 2024 19:34
Copy link
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.

But please squash your second commit into your first so that we have a single commit for this PR.

I just did this for you so that we can submit this and have it for Milestone 3 for the upcoming release.

@HannesWell HannesWell merged commit 969eb9b into eclipse-pde:master Aug 15, 2024
17 checks passed
@peddaiahjuturu
Copy link
Contributor Author

But please squash your second commit into your first so that we have a single commit for this PR.

I just did this for you so that we can submit this and have it for Milestone 3 for the upcoming release.

Thank you @HannesWell. Yesterday was national holiday here, i couldn`t login.

@peddaiahjuturu
Copy link
Contributor Author

i can delete this branch right? and, we can also enable GitHub setting for automatic branch deletion on PR merge right?

@HannesWell
Copy link
Member

i can delete this branch right?

Yes you can.

and, we can also enable GitHub setting for automatic branch deletion on PR merge right?

IIRC there is an option for that. I can check it. But I'm not sure if everybody want's that.

@merks
Copy link
Contributor

merks commented Aug 16, 2024

It’s a setting on one’s own fork.

@peddaiahjuturu peddaiahjuturu deleted the bugs/1374_NPE_after_opening_MANIFEST_file_and_project_deletion branch August 19, 2024 05:09
@peddaiahjuturu
Copy link
Contributor Author

It’s a setting on one’s own fork.

Yes, i can do this on fork @merks .

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.

NPE when open a MANIFEST.MF file in editor and deleting project.
3 participants