Re: Builder methods: should we use setFoo, withFoo, foo, or else?

[Volkan Yazıcı]

I see two kinds of accessor patterns in the wild:

   1. `int getX()` and `setX(int x)` on mutable objects
   2. `int x()` and `withX(int x)` on immutable objects

With records, Java gravitates towards `int x()` and `with { x = x' }`, aka.
withers
.
Scala and Kotlin ecosystems sort of embrace the very same via named
arguments and/or copy methods.

In `release-2.x`, we follow the 1st accessor pattern; whereas in `master`,
we follow both. :facepalm:

I suggest sticking to the 1st approach, everywhere, since

   1. We only have mutable objects
   2. >90% of the project already sticks to the 1st approach
   3. Java ecosystem is dominated by the 1st approach
   4. No backward compatibility issues
   5. No merge conflicts while syncing branches

Maybe we can re-open this subject in the `release-3.x` branch far in the
future.

I have created #1206 
to address this. Feel free to submit a PR.

On Sun, 8 Jan 2023, 22:49 Matt Sicker  wrote:

> In my experience, I’ve usually used withFoo() methods for making immutable
> copies of things while I’ve used setFoo() methods for builder classes as
> those are mutable. In Log4j, we have a mix of these two patterns. I’d like
> us to be more consistent with this.
>
> Which style should we use in Log4j?
> —
> Matt Sicker
>
>

Re: Import order and POM formatting

[Volkan Yazıcı]

For the import order, note that Spotless configuration is aligned with
`.editorconfig` in `log4j-tools`. I find this pretty important and it took
me a while to get them working in the same way. Once we ship Spotless and
`.editorconfig` hand in hand, given we all use IDEA and it has pretty
decent `.editorconfig` support, the transition will be seamless from
maintainers point of view.

I like the `sortPom` idea too – it is not perfect, yet _a rule_ we can
enforce.

Does `sortPlugins` enforce an order in `build > plugins` too? If so, this
is a really bad idea, since there order matters and semantic grouping is
pretty common to make that XML soup digestible.

Long story short, I support both these initiatives. Once you touch these,
would you also make sure all projects (`log4j2`, `log4j-tools`, and
`log4j-transform`)


On Sat, Jan 14, 2023 at 10:52 AM Piotr P. Karwasz 
wrote:

> Hi all,
>
> I noticed a difference in the spotless configuration of our repos.
>
> The `logging-log4j2` repo uses:
>
> java,org,com,\#
>
> The `logging-log4j-tools` repo uses:
>
>
> java,javax,org.apache.logging,,\#java,\#javax,\#org.apache.logging,\#
>
> I copied the latter to `logging-log4j-transform`, so I don't disagree
> with it. However I would like a common rule for all repos, so that I
> can configure my editor accordingly.
>
> Should we move Volkan's order to all other repos? I am not sure if
> `org.apache.logging` imports should be highlighted with their own
> group (it might be unexpected for contributors), but it looks nice.
>
> Another question is POM sorting. In `logging-log4j-transform` I
> experimented with:
>
>   
> false
> true
> true
> scope,artifactId,groupId
> artifactId,groupId
> artifactId,groupId
>   
>
> This is basically what I tried to do in September, the only difference
> is that it does not (and can not) sort `org.apache.logging` artifacts
> before all others. What do you think about introducing the Spotless
> rule to the main repo?
>
> Piotr
>

Re: Requiring signed commits

[Volkan Yazıcı]

It sounds like we have a lazy consensus here.

Piotr, I see your point. I think we need to point this out in a GitHub Pull
Request template
,
which we have none right now. There I also would like to have certain
checks anyway:

- Changelog entry
- Signed commits
- Tests
- etc.

Created #1207  for
this.

On Fri, Jan 13, 2023 at 2:42 PM Piotr P. Karwasz 
wrote:

> Hi Volkan,
>
> On Fri, 13 Jan 2023 at 10:56, Volkan Yazıcı  wrote:
> > *Question:* Shall we require signed commits?
>
> I am obviously Ok with signing, but what should we do with unsigned
> PRs? I would prefer to document the requirement somewhere (in the PR
> template?) so that we don't have to `rebase -f` everything.
>
> Piotr
>