Refactor based on some best practices and idioms (from OpenRewrite recipes)

[lobaorn]

In the same fashion of the previous PR (https://github.com/jakartaee/servlet/pull/561) this one also leverages Moderne for large-scale refactoring, applying many best practices and idioms, like:

Avoid using c-style arrays; Use equals on literals to avoid NPE; Use append at all operations when in String(Builder|Buffer) Removing null assignment on initialization And many others.

Also, reaffirming I have signed the ECA, and since this is a PR focused only on refactoring, I am not sure if other process should be followed.

Use this link to re-run the recipe: https://app.moderne.io/recipes/builder/gau6x2US0

[lobaorn]

As I have addressed through e-mail and on LinkedIn also, this is possible because Tim te Beek of Moderne has enabled the Jakarta repositories there, facilitating applying the recipes (which could be done local, but is much faster and easier through the app.moderne.io interface). I invite you to check @scottmarlow @olamy .

[scottmarlow]

Use this link to re-run the recipe: https://app.moderne.io/recipes/builder/gau6x2US0

I tried opening this link but it wants my github account access, I cannot grant that. Do you have an alternative way for community users to login that doesn't require them to grant access to their github account?

Regarding the changes, I would rather see whitespace changes as a separate pull request but perhaps others will be fine with them.

I'm just curious, have you come across any good tooling for reviewing large pull requests? I know there are more options on the command line but thought I would ask.

I'm really excited and happy about the https://www.linkedin.com/feed/update/urn:li:activity:7125689258756980737/ conversation and look forward to jumping into the conversation soon.

[timtebeek]

I'm just curious, have you come across any good tooling for reviewing large pull requests? I know there are more options on the command line but thought I would ask.

One approach that I'm following with Apache Maven is to run recipes in isolation, to make them easier to review & merge. The changes here are from an aggregate of some ~80 different recipes, which produces a lot of different types of changes and potential for conflicts with other open pull requests. Whereas with recipes in isolation it's easier to spot and fix the most often occurring issues first, before a final pass to get the tail end of issues resolved. Just a suggestion; good thing that automation makes this easy to do.

[lobaorn]

Hi @scottmarlow I asked on the OpenRewrite slack channel about that, but I believe that for now, to use the Moderne app itself it needs a direct connection to the SCM, be it Github or BitBucket. But it is rather just a simplification, since the same result could be obtained by running the recipes locally with maven or gradle.

It is just a lower entry-barrier. But if the login aspect does not have a workaround, then it should be simply the small adjustment of familiarization with running the recipes locally (should be basic).

In this case there were many different set of changes because I ran the following recipes, composing with the "builder" GUI, but that could be made just with a yaml locally also:

• Java API BestPractices (rather small): https://docs.openrewrite.org/recipes/staticanalysis/javaapibestpractices
• Common static analysis issues: https://docs.openrewrite.org/recipes/staticanalysis/commonstaticanalysis
• Static analysis and remediation: https://docs.openrewrite.org/recipes/staticanalysis

Probably it is best to do one at a time, so I will be closing and then opening small PRs.

[lobaorn]

I didn't see your reply Tim, but yeah, I agree, it was more in the sense of demonstrating the possible scale of things, and that is based upon the standard recipes, not even on a crafted one specifically for the TCK/JakartaEE most commom use cases.