Snippets: reduce warnings

eclipse-platform / eclipse.platform.swt

Eclipse SWT

https://www.eclipse.org/swt/

Eclipse Public License 2.0

101 stars 123 forks source link

Snippets: reduce warnings #1277

Closed jukzi closed 2 weeks ago

jukzi commented 3 weeks ago

The static method should be accessed directly
The static field should be accessed directly
discouraged access

github-actions[bot] commented 3 weeks ago

Test Results

442 files ±0 442 suites ±0 9m 30s :stopwatch: -38s 4 126 tests ±0 4 118 :white_check_mark: ±0 8 :zzz: ±0 0 :x: ±0 16 318 runs ±0 16 226 :white_check_mark: ±0 92 :zzz: ±0 0 :x: ±0

Results for commit 472eed8a. ± Comparison against base commit 4c4074f6.

HannesWell commented 2 weeks ago

The static field should be accessed directly

In https://github.com/eclipse-platform/eclipse.platform.swt/pull/1253 actually I disabled that warning assuming it was written like that to increase readability.

jukzi commented 2 weeks ago

In #1253 actually I disabled that warning

I see problem marker for this in workspace? disabling would be OK for me too, but seeing a Problem is not appropriate when it can just be fixed with quickfix. I don't see a difference in readability.

HannesWell commented 2 weeks ago

In #1253 actually I disabled that warning

I see problem marker for this in workspace? disabling would be OK for me too, but seeing a Problem is not appropriate when it can just be fixed with quickfix.

Aren't these info-level markers issued by Sonar-Lint? But these could also be info-markers issued by JDT (haven't checked it).

I don't see a difference in readability.

I haven't looked in detail into the snippets and don't have a strong opinion. I just kept it as it is when unifying the preferences because I assumed the original authors found it better the way it is. Since these snippets are for consumers of SWT I think the code-style should be a good example. If we think qualifying the static access always with the declaring class I'm fine either.

But then the marker-level should be increased to warning to not add new issues.

jukzi commented 2 weeks ago

Aren't these info-level markers issued by Sonar-Lint?

I don't even have that installed and you can look in your workspace's Problems view.

But then the marker-level should be increased to warning to not add new issues.

Well it was your commit that set org.eclipse.jdt.core.compiler.problem.indirectStaticAccess=info on all related projects. Before it was ignore/warning depending on project. What i do not understand is why info markers are not visible in jenkins. And why you modify warning levels without looking in the Problems view ;-) and why you submitted with +1 new warning, so did even ignore jenkins warnings. Anyway i think your "info" level is fair as static access is not any real problem. And linking the settings was a good idea.