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

Only consider existing artifacts for the classpath #1682

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

laeubi
Copy link
Member

@laeubi laeubi commented Feb 17, 2024

Currently only if the artifact file is null it is not considered but if the file does not exits JDT complains. It even seems there was once in a time such a check but it is disabled, this is an attempt to check if we can reenable it again.

@laeubi laeubi requested review from fbricon and HannesWell February 17, 2024 07:18
Copy link

github-actions bot commented Feb 17, 2024

Test Results

  214 files  ±0    214 suites  ±0   16m 3s ⏱️ - 1m 11s
  664 tests ±0    649 ✅  -  5  10 💤 ±0   5 ❌ + 5 
1 328 runs  ±0  1 296 ✅  - 10  22 💤 ±0  10 ❌ +10 

For more details on these failures, see this check.

Results for commit 2bf3489. ± Comparison against base commit 94fa546.

♻️ This comment has been updated with latest results.

Currently only if the artifact file is null it is not considered but if
the file does not exits JDT complains. It even seems there was once in a
time such a check but it is disabled, this is an attempt to check if we
can reenable it again.
@HannesWell HannesWell force-pushed the only_consider_existing branch from 39e717f to 2bf3489 Compare February 17, 2024 09:49
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.

Looks like there are test-failures now. Don't know if they are only due to changed circumstances or are really meaning full.
It would be good to add a test for the case you described.

Sadly the commented out line goes back until the Initial commit at Eclipse. Would have been nice to know why this was disabled. :/

@laeubi
Copy link
Member Author

laeubi commented Feb 17, 2024

@HannesWell yes thats why I have disabled it to see what / if test fails, sadly some tests are written wit some numbers of problems in the expectation without looking at the content.

At least it show it has an effect .. but probably not something for the current release.

@HannesWell
Copy link
Contributor

sadly some tests are written wit some numbers of problems in the expectation without looking at the content.

Yes, that's what I meant with non meaningful tests :)
One should check each failing tests if its just the number of expected CP-entries that has to be adjusted or if the failure is really revealing a potential problem.

At least it show it has an effect .. but probably not something for the current release.

Yes, lets keep this for the next cycle.

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.

2 participants