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

Reduce "Discouraged access" warnings #989

Merged
merged 2 commits into from
Dec 7, 2023

Conversation

jukzi
Copy link
Contributor

@jukzi jukzi commented Dec 6, 2023

No description provided.

Copy link

github-actions bot commented Dec 6, 2023

Test Results

     270 files       270 suites   42m 31s ⏱️
  3 329 tests   3 299 ✔️ 30 💤 0
10 284 runs  10 194 ✔️ 90 💤 0

Results for commit 83e5af9.

♻️ This comment has been updated with latest results.

@jukzi
Copy link
Contributor Author

jukzi commented Dec 6, 2023

why is marked as fail while there are no fails?
image
image

@laeubi
Copy link
Contributor

laeubi commented Dec 6, 2023

Because you added one new compiler warning (and yes the mention of test failure is WRONG please report it to the author of the Jenkins plugin):

grafik

Also you clicked already on https://ci.eclipse.org/pde/job/eclipse.pde/job/PR-989/1/ you see a small orange warning here:
grafik

Compare for example with something that has found issues but no new ones:
grafik

@jukzi
Copy link
Contributor Author

jukzi commented Dec 6, 2023

Because you added one new compiler warning (and yes the mention of test failure is WRONG please report it to the author of the Jenkins plugin):

Thanks for the explanation. can it be configured, that a reduction of warnings (7 reduced here, 1 was just moved) does not count as error?
image

(and yes the mention of test failure is WRONG please report it to the author of the Jenkins plugin):

who's that author? or the used plugin, please

@laeubi
Copy link
Contributor

laeubi commented Dec 6, 2023

The configuration options are here but I think one can't exchange one warning for another.

The plugin that publishes Warnings from Jenkins > Gitbub is this here, the build itself seems to be contributed by this.

Of course if one think it should do that one can still merge.

@jukzi jukzi force-pushed the Discouraged_access_reduce branch from e0c2050 to 32cf00b Compare December 7, 2023 07:44
akurtakov added a commit to akurtakov/eclipse.pde.ui that referenced this pull request Dec 7, 2023
in XmlReferenceDescriptorWriter .

Chained complain on eclipse-pde#989
akurtakov added a commit that referenced this pull request Dec 7, 2023
in XmlReferenceDescriptorWriter .

Chained complain on #989
EcljpseB0T and others added 2 commits December 7, 2023 13:00
The params documented in the javadoc make no sense with actual params.
@akurtakov akurtakov force-pushed the Discouraged_access_reduce branch from 19d949d to 83e5af9 Compare December 7, 2023 11:00
@akurtakov
Copy link
Member

It would be a bit noisy initially as it will require extra fixes in order to not produce new warnings but over time such cases should be rarer and rarer aka I'm more and more convinced this is good path forward.

@akurtakov akurtakov merged commit 54dc1f1 into eclipse-pde:master Dec 7, 2023
15 checks passed
@jukzi jukzi deleted the Discouraged_access_reduce branch December 7, 2023 11:32
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.

4 participants