Replies: 22 comments 57 replies
-
|
example: #5635 |
Beta Was this translation helpful? Give feedback.
-
|
Thanks for the suggestion. I think, it makes sense to add this config. But we need to mention it in the documentation, as not all IDEs/Editors use it by default. Some (like VS Code) need a plugin. So the relevant documentation needs to be updated:
Also, if we have this config, then I assume, we don't need e.g. the IntelliJ Specific config anymore? |
Beta Was this translation helpful? Give feedback.
-
Yes I think so too, but @romani has different opinion. We will find out after migration, if we still need the file for manual config. @Anmol202005 ATM its The setting-up-checkstyle file is needed, as it will evolve into the new |
Beta Was this translation helpful? Give feedback.
-
|
What about Spotless? How is it related (or not) to editorconfig? Can you please explain how all of those fit together and how you think we should proceed with migration to a new set of code quality plugins? Can we remove checkstyle? Are there any things that we use in checkstyle and that are not supported by spotless? How do we keep the spotless formatter synced with IDE configs? And how is a developer supposed to use these plugins? I wouldn't want the default behavior of I think we should answer those questions and finish this discussion before we merge or even review your PRs @Pankraz76. Your PRs #5846 #5845 #5636 and #5776 are huge and overlap, which make them difficult to review. The main problem is however that you haven't explained anywhere, in detail, what you're trying to do or how you want to get there. You should do that before you open PRs, especially if they are numerous and large. Please take your time to write your case here; in the meantime I will close your PRs. |
Beta Was this translation helpful? Give feedback.
-
|
ah nice, thanks yes. The topic does not change. Only the somehow, as constantly evolving, irrelevant impl. details. I have overcome myself, thanks to the learnings of maven and quarkus. As we already agreed upon, fixing things once and for everybody, we dont need this plugin anymore as spotless is the successor, automating this burden.
Long story short, in order not to reinvent the wheel, we should leverage the convention over configuration principle and follow big bother maven, at least for You need to check if its worth to fix the eclipse config or be smart like maven let palantir to the job. https://github.com/apache/maven-parent/blob/f4ab7c2eb04865071186c57a263243c72b6e8d52/pom.xml#L1219 Starting with the simple integration of this plugin, on trivial topic meaning imports, then build up from there, as everything is then possible. Yes its impossible to review this of course, as we have countless flaws, which is the natural outcome, when not having auto correction like this. Nobody can to this kind of job. Except for one. Once we have implemented, we do not need checkstyle anymore, as its 1 LOC activating ether eclipse custom config or google or palantir config, this is some detail the community needs to decide. Its not important to me, assuming nor for others, we only care about convention in general, not in detail, naturally evolving to auto fix. When having ability to control, code becomes fluid. One of you guys the will then just add 1 LOC activating the change. Then you check (locally) the pattern, as we can not check 1.200 header files in detail as discovered in:
As these changes are ALL about pattern they are "easy" to review as all the same. Humans maybe error prone, but master of disaster, tend to pattern and natural symmetry considering PI 3.1415926535..... You simply have to trust the tool it will execute itself and validate just like the maven build will fix itself. |
Beta Was this translation helpful? Give feedback.
-
the good thing is they are done one time only, then they are fixed by design/devinition. new feat incoming, as this would address a lot of flaws, ideally all, as spotless will check and apply all the time. |
Beta Was this translation helpful? Give feedback.
-
|
Its all related and spot the solution to all:
considering this a good PoC and realistic to merge. Then could increment from there even up to fully replacing checkstyle, automating all its features. |
Beta Was this translation helpful? Give feedback.
-
|
Whatever the code style we pick I want an Intellij config that will format the code to be compliant code. And Maven's CI profile should only check for compliance without changing any code. Ideally Maven wouldn't do any rewrite unless prompted explicitly. Maybe while we're at it we could have a git pre-commit setup that calls the formatter? Wdyt @adangel
If using Spotless is the solution to everything please stop opening new issues about it, we're already talking about it here. Let's discuss it here and stop dispersing. |
Beta Was this translation helpful? Give feedback.
-
This could be complex, as Spot takes some time, while committing is a fast and frequent task. Assuming every commit would then somehow execute Despite that, I’ve experienced in some projects that hooks failed to ensure Conventional Commits. They were installed in my Git but still not enforced. Yes, having it run in the cloud only makes sense for the We should be able to detect runs on a dev machine and only execute the This would also reduce the need for manual interaction by executing some dedicated profile. Quarkus build has a parameter Maven and Quarkus both deliver the fixes "out of the box," executed silently when passing the
This is very specific but understandable, assuming we all use IDEA. Its all there, we just need to use is. |
Beta Was this translation helpful? Give feedback.
-
|
maven does not apply auto fix on cloud like expected:
in gradle its one simple if and its done: if (System.env['CI'] == null) {
tasks.named('spotlessCheck') {
dependsOn 'spotlessApply'
mustRunAfter 'spotlessApply'
}
} |
Beta Was this translation helpful? Give feedback.
-
|
I'm going to change this issue to a discussion topic and change the subject to "Improve Developer Experience". |
Beta Was this translation helpful? Give feedback.
-
|
I've summarized now the discussion - look at the top. I'd suggest, if you have more ideas, we can extend the list. No offense, @Pankraz76, but it seems to me, that you started out with "I want to use spotless" (which is the solution), before you looked at which problem (what is itching you?) you are trying to solve. Going on on this topic, I suggest we discuss it here and use the discussion functionality here - so: for each problem/itch/topic, a new comment on which everybody can reply directly to the topic, so that we avoid the mix of different things like in the comments above. |
Beta Was this translation helpful? Give feedback.
-
kind of yes, kind of no. Spot seems the natural alpha, having 6k stars, being used by maven-patent. Its actually just a wrapper for others like google-java-format, and as this is already dead, shifting again to palantir.
yes sorry im overpacing very easily, even tho this is of course not my intend you overwelm. |
Beta Was this translation helpful? Give feedback.
-
Topic: License headers
|
Beta Was this translation helpful? Give feedback.
-
|
To me it seems that basically everything, meaning the very basics, is very broken, not even having import config or header config, to make ppl. ask lousy ass question, like me, what config should be used. Should I use double start * headers on full moon nights, or whats the special crazy rule here again, as having SSOT seems not to be prior. |
Beta Was this translation helpful? Give feedback.
-
Yes, I agree.
Don't allege something you can't know. I don't remember you back then. The original reason, I don't remember exactly, but something along the line, we don't care, how the license header exactly looks like, as long it is there and contains the information. And we didn't spend time to align the existing code base.
Auto-fix is not needed IMHO for such a switch. You do it once for all existing files in the repositories. The only question is, how to deal with new files, more specifically: how is the license header added into new files?
Imports is a different topic - this thread is about the license headers. Please stay focused. Having the build failing for a missing/wrong license header is not a pleasant experience, agreed. But having to run spotless:apply only helps, if it is integrated automatically if you run locally "mvn verify". Otherwise you always need to run it automatically. Then you shouldn't forget to commit the possible changes made by spotless. But again, even better would be, if your IDE would already take care of this. In summary, I think we have now one argument in favor for switching from checkstyle to spotless: it's spotless:apply. Are there other features, that are missing in checkstyle? We should probably open here a new thread for collecting reasons, why to switch from checkstyle to spotless (and the argument, that everybody else uses it, is not an argument: if we switch to spotless, it should solve a problem for us, otherwise it's non-sense).
Agreed. And you identified two inconsistent/contradicting things (license header and - warning off-topic in this thread - import order).
The check is as solid as we configure it. The two license header variants where explicitly allowed. It's not a problem of the tool, that checkstyle would be brittle here. We configured it to be lax on the license headers.
For me, the single source of truth regarding this is: the build with "mvn verify" runs successfully. That does all the checks. It runs independently of IDEs. If the IDEs format code in a different way, then the IDE configuration is broken.
That's a good point - ideally it would be "simple" (whatever the definition of simple is) to setup your development environment, start coding and you don't need to bother about configuration. That's the topic developer experience.
That's what I did today. But remember, one step at a time. I did explicitly not introduce spotless, but just fixed the license header and enforced it so that when whoever adds a new file, the license header is the same. The issue with the license header is independent of the question, whether we switch or we don't switch to spotless. Also note: we don't have just pmd/pmd, we have also other repositories.
|
Beta Was this translation helpful? Give feedback.
-
|
I've created now the PRs:
Once these are done, we can close this topic and move on thinking about tool changes. |
Beta Was this translation helpful? Give feedback.
-
so you want to first fix the issues, then migrate the tool, which is important imho, as even if check is working fine, its still deign to annoy not designed to fix. Just think about junit: Its not about some random impl. detail wether its imports, licence header, or whitespaces. Its about having obsolete stuff begin fixed out of the box, instead of imposing this burden to the contributor. We should fully leverage the ability of computer not only to point our, but go beyond and apply the own suggestions. This looks like the minimum config, we would need to apply the style we want to enforce, but currently miserably fail to do so. Even tho check would be able to fix it, its still not configured properly allowing inconsistencies, which checkstyle PMD and spot it all about to prevent these. |
Beta Was this translation helpful? Give feedback.
-
Sorry, I don't understand this. Do you mean, checkstyle doesn't work? Please give concrete examples... which inconsistencies are allowed? Just to make sure - I'm not against switching tools or adding tools. But I want to understand why... |
Beta Was this translation helpful? Give feedback.
-
Topic: Switch to spotless or not?
Things to consider:
|
Beta Was this translation helpful? Give feedback.
-
both have plugins Its very crazy to see whats possible. Ty to checkout https://github.com/palantir/palantir-java-format yourself. This project has even auto format on save actions automated out of the box as they use some IDEA plugin. Here its really an ease to work on as everything is fixing itself upfront. |
Beta Was this translation helpful? Give feedback.
-
|
Thanks, good to know, there is also a IDE plugin. Do you have it installed? any experiences? |
Beta Was this translation helpful? Give feedback.
-
No, I only rely on the build. Expecting this to work find as it looks up to date. |
Beta Was this translation helpful? Give feedback.
-
Topic: import order inconsistenciesissue: import order is not consistent between IntelliJ IDEA / Eclipse / Checkstyle Config
What is the import order enforced by checkstyle currently? Groups:
What is the import order in the eclipse configuration currently? https://github.com/pmd/build-tools/blob/main/eclipse/pmd-eclipse-imports.importorder Groups:
What is the import order in the IntelliJ IDEA configuration currently? Groups:
What is the import order in the Netbeans configuration currently? https://github.com/pmd/build-tools/blob/main/netbeans/formatting.zip Groups:
not sure, where static imports would be here... |
Beta Was this translation helpful? Give feedback.
-
|
yes if you can accept the licence. Please pay special attention to maven and quarkus escalation.
Indeed, rewrite is the natural evolution, as this goes beyond styling and fixing real issues! Considering junit: |
Beta Was this translation helpful? Give feedback.
-
|
as spot limited its therefore simpler to integrate and gain feedback of this automated approach. If theres then an internal update with an better tools, this will be a smaller change for the users. We could start simple and then increment from there, in the sake of |
Beta Was this translation helpful? Give feedback.
-
|
If OpenRewrite already supports Checkstyle configurations, then we don't need to "start small and increment from there", since it already supports our code style. I don't see why we would use another tool like spotless and create yet another config that is not in sync with checkstyle or the IDE configs. Overall fixing issues automatically is just a developer convenience, as it won't be used in CI. And I don't think it's a high priority feature or something we terribly need -the IDE configs already do the job. So I think adding a feature like that should be very low effort and more importantly, very low maintenance. We shouldn't add another configuration to maintain. In that sense if there is an automated rewrite tool that supports one of the configurations we already have (like the Checkstyle config), then we should go for this and not bother with other tools like Spotless. |
Beta Was this translation helpful? Give feedback.
-
|
Yes good argument. Will try this then, as last time it was not working as expected. |
Beta Was this translation helpful? Give feedback.
-
|
broken as expected but they will fix then we will see. |
Beta Was this translation helpful? Give feedback.
-
spotless:sortpom
|
Beta Was this translation helpful? Give feedback.
-
|
thanks. The pom seems not to enforce some format, currently this can simply be done by check always sorting the dependencies and stuff. |
Beta Was this translation helpful? Give feedback.
-
|
But what's the issue with that? I want you to explain it to me, please. Why is it a problem for you (and maybe others)? |
Beta Was this translation helpful? Give feedback.
-
IDE agnostic configuration
@Pankraz76 We already have a codestyle. What's the problem with that codestyle? Is there something missing? your comment #5867 (comment) seems to expect some things, that are not defined in our codestyle yet. Also, if we use spotless, which tool makes sure, that the IDE configuration is aligned? (to be honest, in the end everybody is using some IDE) |
Beta Was this translation helpful? Give feedback.
-
only issue is, missing no auto fix capability. The style itself does not matter to me. Only that we have to invest time on obsoletes. |
Beta Was this translation helpful? Give feedback.
-
|
Ok, in summary: You don't want to spend time on code style issues. Fair enough. We already have documented in the Guide, how to configure the code style in the IDE of your choice. We now also fixed the bug in the configuration for eclipse (thanks for noticing). If a contributor follows the Guide, then I assume, everything is ok? Can you please verify that assumption first? Then we could go on and think about, how to make it easier to have a properly configured IDE. |
Beta Was this translation helpful? Give feedback.
-
= configuration the best configuration is called a convention, making the whole approach and discussion obsolete as we have convent over config. |
Beta Was this translation helpful? Give feedback.
-
Towards "convention over configuration"
@Pankraz76 I think, that is the core of your whole PRs/issues (and hence I named this whole discussion "Improve Developer Experience"). Is this correct? All the other topics here are probably only some small steps/parts, that help, but don't fulfill the vision fully. If you would need to write a contributors guide for PMD, what should it look like? In theory, our starting point is currently https://github.com/pmd/pmd/?tab=readme-ov-file#-contributing - that's the assumption, that anybody who wants to contribute is at least reading this section in the readme... |
Beta Was this translation helpful? Give feedback.
-
|
Difficulty with Documentation (I struggle with reading and writing, as you can tell). I tend to avoid guides and tutorials because they’re often outdated or incorrect—investing time in them feels like a waste. This isn’t entirely surprising; every thing (doc and code) naturally becomes obsolete over time. A clear example is the broken import configuration that persisted for seven years without correction. That’s why automated tools—like Spotless and others —are so valuable. They can detect and even fix issues proactively, reducing reliance on unreliable or aging resources. |
Beta Was this translation helpful? Give feedback.
-
|
By the way, I don't remember you mentioning this: @Pankraz76 which IDE are you using actually? |
Beta Was this translation helpful? Give feedback.
-
|
of course i use IDEA like every professional. Im actually pretending to use vi, vim or notepad++. When having a proper build fixing itself, the mandatory Just let the build fix itself and everything else becomes obsolete, right? |
Beta Was this translation helpful? Give feedback.
-
Or configure your IDE to automatically fix it already for you and then the build won't fail for codestyle issues.
But that's not realistic. As soon as you want to do serious work, you should use the correct tool. |
Beta Was this translation helpful? Give feedback.
-
convention is the new config. Having a SSOT config, rolled out as a convention, for everybody else is just a very nifty approach. Tell me what happens if you going to change the import order? Then EVERYBODY has to update their setup. Is this the right way to do so? Or should we just roll out the new version? Its like we convert the guide into a living document as its really doing and not just pretending or requesting. |
Beta Was this translation helpful? Give feedback.
-
|
PMD feels outdated compared to modern tools like Maven, Quarkus, and Spotless, which have moved beyond cumbersome configuration requirements for basic formatting. While I appreciate PMD as a tool, its insistence on manual whitespace configuration—rather than adopting modern conventions—makes it feel obsolete, especially when alternatives like Rewrite automate these tasks effortlessly. I don’t contribute much to PMD, not because I dislike it, but because its maintenance burden outweighs its benefits in its current state. That said, I still recognize PMD’s importance and believe it deserves an up-to-date build setup. The current configuration, which some defend, was effectively broken for seven years. Is this the quality standard we expect from an established tool? Instead of clinging to outdated practices, we should move decisively in the direction already set—automating what should be convention, not configuration. If we agree on the goal, why the delay? Let’s be strict and consistent in modernizing PMD rather than preserving inefficiencies. |
Beta Was this translation helpful? Give feedback.
-
im flexible and my page is big. you just on the top mountain laying on my foundation page, so we are on the same trust me. I know your needs but i see also the insides as well as having the experience with other projects.
by bot ONLY. The PR´s containing worthy contributions, not only by me of course, even new features are just ignored.
Ideally we will end up like palantir have build checking and auto configure idea, no need to configure any. Then we end up with
|
Beta Was this translation helpful? Give feedback.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
-
This is still vague. What IDE plugin are you talking about? Do you mean IDE plugin or IDEA plugin? Do you really mean "rewrite" or OpenRewrite? Please use the correct names otherwise it is difficult to understand you. Ideally we wouldn't lock developers into Intellij, especially not Intellij Ultimate. And AFAIU it still doesn't help with the goal of easing IDE configuration, as the plugin does not replace the IDE formatter anyway. So we still need to carry a format file. It seems the OpenRewrite solution doesn't really help except to apply formatting on the command line. Are the problems with google java format/ palantir really serious? They are depended on by a lot of people. If they don't have a lot of feature updates this is likely because they are mature already, not because they are abandoned. I wouldn't discard them so far. We still need to list out the checks from Checkstyle that would need an alternative. |
Beta Was this translation helpful? Give feedback.
-
yes, as we aim for but fixes as well, theres might gonna be an compromise to make, to achieve this. Maybe the plugin is possible to run only one file, like checkstyle does then it might be possible to have it similar. |
Beta Was this translation helpful? Give feedback.
-
|
Having the same eclipse formater config, we should be able to apply the same approach like open search. Effectively automating checkstyle, assuming the rule match.
|
Beta Was this translation helpful? Give feedback.
-
|
kind of the same topic: |
Beta Was this translation helpful? Give feedback.
-
|
I don't understand why you would elevate the eclipse code format to the standard format that every one should use in this project now. Most developers here have not used this format, as is evidenced by the large code delta. So why would we sanctify this instead of going with a more conventional format, like Google or Palantir format? Also in #5925 you are removing Checkstyle without having done the compatibility analysis that @adangel and I have been asking for since the start of this discussion thread. Last thing, I don't think it is a good argument to say that these tools are used by big projects, when you are the one opening PRs about it (and your PRs usually get rejected). This is circular reasoning. |
Beta Was this translation helpful? Give feedback.
-
|
Yes. How about we have a quick discussion and then reach a conclusion? The arguments are clear and all valid. What if we schedule a short call to demo the proposed changes (as intended in the PRs) and then determine what’s actually needed for PMD? A 30-minute timebox should be more than enough. This is primarily a strategic decision for the dev team to define—I’m just providing the tooling/capability. |
Beta Was this translation helpful? Give feedback.
-
|
I don’t think a live call would be productive. If your proposal can’t be made clear and convincing in writing, especially after this much discussion, then I don't believe a meeting would change that. Unless the concerns that have been repeatedly raised (eg compatibility analysis, clear characterization of your changes) are addressed in writing, I don’t see a way forward. |
Beta Was this translation helpful? Give feedback.
-
|
I understand your concern. Since I don’t fully grasp the details of your issue yet, I’d like to start small. We can identify your biggest needs by working through practical examples from several live projects. |
Beta Was this translation helpful? Give feedback.
-
|
closing to stop starting start finishing. |
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
-
Collections if things that can be improved ("what is itching you")
importOrderwithMaven Spotless Plugin#5776checkstyleburden imposed ontrimTrailingWhitespaceandremoveUnusedImportswithMaven Spotless Plugin#5848editorconfig.org-editorconfig-maven-plugin#5636 and [core] add IDE agnostic configuration #5651 and [core] add IDE agnostic configuration withMaven Spotless Plugin#5867/**and some with block-comment/*Tools/Config/Practices, that are currently used in PMD
mvn verifylocally ensures, everythings compliesTools/Config/Practices, that maybe offer improvements
Note: These need to be evaluated, whether we really want to use them...Do they offer so much more in comparison to the tools we already use?
Maven Spotless Plugin#5867)General considerations
Old info
Addressing the comment and issue of
people use a lot of editors and very different versions, which is a valid concern—one good approach to handling this is using Spotless.Applying a strictly enforced style can be challenging without a universal solution. Spot provides a way to apply the config out of the box, making it easier to maintain consistency.
This is very specific but understandable, assuming we all use IDEA.
Its all there, we just need to use is.
Beta Was this translation helpful? Give feedback.
All reactions