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

Add option to disable m2e problem reporting #2037

Closed
Marcono1234 opened this issue Feb 7, 2024 · 5 comments · Fixed by #2059
Closed

Add option to disable m2e problem reporting #2037

Marcono1234 opened this issue Feb 7, 2024 · 5 comments · Fixed by #2059

Comments

@Marcono1234
Copy link
Contributor

Marcono1234 commented Feb 7, 2024

Version

  • OS: Windows 10
  • Maven Plugin: 2.43.0
  • Eclipse: 2023-12 (4.30.0)

Description

Relates to #1814

It seems the m2e formatting issue reporting (#1962) is enabled by default and cannot be disabled. The way these issues are reported is in my opinion a bit problematic:

  • They are reported as errors with a big red icon
  • The description of the error can be difficult to read and understand (even to understand that this is related to Spotless), especially for larger sections

Example:
Error screenshot

Even if this can be adjusted or improved, e.g. by using a different severity (if possible) and making the message more concise, it might in some cases be useful to completely disable this in the plugin configuration. For example for the use case of letting users write the code in whatever formatting they want (without requiring them to adjust their IDE), and then only requiring running mvn spotless:apply once in the end. For that case, these errors during development might just be distracting and might actually scare away new contributors to the project using Spotless.

Example project

https://github.com/google/gson

Writing code in Eclipse IDE which does not match the Google Code Style shows multiple Maven errors due to formatting violations.

@Marcono1234
Copy link
Contributor Author

Maybe there is also a bug with the error reporting that these errors just accumulate without ever being cleared, as long as there is still any formatting violation at that line.

@kwin
Copy link
Contributor

kwin commented Mar 2, 2024

Indeed there is an issue with not removing old error markers. I will try to come up with a fix. This is a regression of #1962. In addition allowing to disable m2e execution (and therefore prevent emitting of errors within Eclipse) and allowing to tweak the severity of message markers in incremental builds via another plugin parameter should be possible to support as well.

@Marcono1234
Copy link
Contributor Author

Thanks a lot for looking into this!

I am not that familiar with Eclipse's internals, m2e and Maven incremental builds, so I have some questions regarding your proposed changes:

  • Are there cases where Eclipse / m2e performs a non-incremental build and these error markers could still occur inside Eclipse?
    Please don't get me wrong, these Eclipse error markers can probably be quite useful for projects where all contributors are aware of the build setup and the Spotless usage.
    But for public open source projects where new contributors might not know anything about the build setup (or might have never used Spotless) I think these error or warning markers in Eclipse can be confusing and it would be useful to disable them completely. And instead let users check and fix Spotless violations at the end with mvn spotless:....
  • Do your proposed changes affect non-m2e builds as well?
    For example, could it happen that users run mvn ... and Spotless is not executed and would miss formatting violations due to this?

@kwin
Copy link
Contributor

kwin commented Mar 2, 2024

Are there cases where Eclipse / m2e performs a non-incremental build and these error markers could still occur inside Eclipse?

No, they are only emitted for incremental build

Do your proposed changes affect non-m2e builds as well

No!

@Marcono1234
Copy link
Contributor Author

Marcono1234 commented Mar 2, 2024

Do your proposed changes affect non-m2e builds as well

No!

Thanks for the clarification. I just wasn't sure because the condition in AbstractSpotlessMojo.shouldSkip() looked like it is general Maven API and not something m2e / Eclipse-specific.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants