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.

Sign up for GitHub

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

Jump to bottom

Track whether the manifest of a bundle component has an explicit EE #1292

Merged
merged 1 commit into from
Jun 6, 2024
Merged

Track whether the manifest of a bundle component has an explicit EE #1292

merged 1 commit into from
Jun 6, 2024

Conversation

merks
Copy link
Contributor

@merks merks commented Jun 6, 2024

@merks merks requested a review from iloveeclipse June 6, 2024 12:35
@merks
Copy link
Contributor Author

merks commented Jun 6, 2024

@iloveeclipse

I've verified that this fixed the problem where the baseline version and the workspace version as the same and now both are validated such that both indicate they have no BREE and hence to BREE change:

image

Also verified that if I were to add an explicit BREE, this would be detected as a change to the BREE:

image

@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 6, 2024

Test Results

  285 files   -     6    285 suites   - 6   48m 40s ⏱️ - 12m 4s
3 578 tests ±    0  3 500 ✅  -     2   76 💤 ± 0  1 ❌ +1  1 🔥 +1 
8 328 runs   - 2 703  8 119 ✅  - 2 681  207 💤  - 24  1 ❌ +1  1 🔥 +1 

For more details on these failures and errors, see this check.

Results for commit a41eb46. ± Comparison against base commit 8e177ee.

@merks
Copy link
Contributor Author

merks commented Jun 6, 2024

@iloveeclipse

I'll give some more time to review, but I'd like to commit this today so that I can update my IDE to fix the problem tomorrow....

iloveeclipse
iloveeclipse approved these changes Jun 6, 2024
View reviewed changes
Copy link
Member

@iloveeclipse iloveeclipse 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 for me and I also can confirm that the API error on Linux on SWT is gone with this patch.

@iloveeclipse
Copy link
Member

I'll give some more time to review, but I'd like to commit this today so that I can update my IDE to fix the problem tomorrow....

Sure. I've checked the patch on Linux, sounds reasonable, but I'm not that deep in this PDE code. I would say we should merge to get rid of the current (false positive) error on SWT and if the patch could be improved, it should be improved later.

@iloveeclipse iloveeclipse merged commit af95448 into eclipse-pde:master Jun 6, 2024
15 of 17 checks passed
@merks merks deleted the issue-1283 branch June 6, 2024 14:21
@iloveeclipse iloveeclipse mentioned this pull request Sep 4, 2024
Closed
HannesWell added a commit to HannesWell/eclipse.pde that referenced this pull request Sep 5, 2024
@HannesWell
Restore check if a BundleComponent has explicitly declared its EE
e04c343 
And assume no EE if that check is positive.

Restores main part of
eclipse-pde#1292
Fixes eclipse-pde#1386
HannesWell added a commit that referenced this pull request Sep 6, 2024
@HannesWell
Restore check if a BundleComponent has explicitly declared its EE
951532e 
And assume no EE if that check is positive.

Restores main part of
#1292
Fixes #1386
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iloveeclipse iloveeclipse iloveeclipse approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@merks @iloveeclipse