Bump io.takari:takari from 51 to 55 #303

cstamas · 2024-05-23T19:29:13Z

This involves full reformat of the sources using spotless maven pluging. If this commit enters master, it MUST be followed by another one that sets this commit hash as "git blame ignore".

Changes:

POM parent uses takari-pom 55
no any other source change
format of java, scala and kotlin sources

Fixes #262 implicitly, by a minor cleanup among deps

cstamas · 2024-05-23T19:29:50Z

@lefou @headius WDYT?

lefou · 2024-05-23T19:32:49Z

Lets wait for CI. I suspect removed support for Java 8.

cstamas · 2024-05-23T19:41:20Z

Yes, spotless-maven-plugin is 11+ so it needed Java 8 hacks (to NOT run)

lefou · 2024-05-23T19:43:24Z

I think spotless can also format Scala code, but I have no idea how to enable it. If it is based on scalafmt, I can provide a reasonable scalafmt.conf though.

lefou

Looks good to me.

cstamas · 2024-05-23T19:48:01Z

So, this is the "price" of latest Takari parent POM: spotless (palantir) formatted source code, and build will fail if unformatted code is being built: to reformat mvn spotless:apply (but you need Java 11+).

cstamas · 2024-05-23T19:48:37Z

Personally I love this format, whole Maven 3.9 and 4 and all plugins went to this source format as well. POM are formatted as well (element order BUT NOT like "sortpom" that is very bad idea, as dep ordering DOES MATTER).

lefou · 2024-05-23T19:50:05Z

So, this is the "price" of latest Takari parent POM: spotless (palantir) formatted source code, and build will fail if unformatted code is being built: to reformat mvn spotless:apply (but you need Java 11+).

Sounds good to me. Formatted code makes maintenance and PR management easier!

cstamas · 2024-05-23T19:51:47Z

Lets give some time to @headius and others to chime in...

lefou · 2024-05-23T19:57:13Z

If you want to enable spotless for scala files, you could use this config file.

cstamas · 2024-05-23T20:07:55Z

@lefou done, pls review

lefou · 2024-05-23T20:10:54Z

polyglot-scala/pom.xml

+        <artifactId>spotless-maven-plugin</artifactId>
+        <configuration>
+          <scala>
+            <scalafmt />


I think you need to refer to the .scalafmt.conf here. Maybe, because it's not in the repository root directory. The format of the files doesn't match the config.

Yup, realized that, but conf file needed change: error was:

Caused by: java.util.NoSuchElementException: version [expected 3.7.3 or 3.7.3]: 3.7.15

lefou · 2024-05-23T20:13:11Z

polyglot-scala/.scalafmt.conf

+# Newer versions won't work with Java 8!
+version = "3.7.15"


Suggested change

# Newer versions won't work with Java 8!

version = "3.7.15"

version = "3.8.1"

Whatever else is here than "3.7.3" the plugin pukes, with error above, unsure why.

Maybe, we need an explicit version in the plugin config?

<version>3.8.1</version>

But I think we can live with that older version too.

You are right, set for wanted 3.8.1

lefou

Looks reasonable.

lefou · 2024-05-23T20:19:48Z

polyglot-scala/src/main/scala/org/sonatype/maven/polyglot/scala/ScalaMapping.scala

- * are made available under the terms of the Eclipse Public License v1.0
- * which accompanies this distribution, and is available at
- * http://www.eclipse.org/legal/epl-v10.html
+ * Copyright (c) 2012 to original author or authors All rights reserved. This


This looks a bit ugly and is unexpected (the merged lines). Maybe, because it was formatted without any config before? Not a showstopper, but maybe you could retry with the original files?

Seems you were right, re-reformatted with master checkout first and looks better

Very nice. 👏

headius · 2024-05-23T21:00:37Z

Worth it for fixing 2-space indent on all the Java files alone. 👍

This involves full reformat of the sources using spotless maven pluging. If this commit enters master, it MUST be followed by another one that sets this commit hash as "git blame ignore". No code change, just reformat of Java, Scala and Kotlin source.

cstamas · 2024-05-24T10:11:41Z

Now, Java, Scala and Kotlin sources are formatted. Will let this PR one more day (to get possible feedback from any interested party) and will merge after.

cstamas · 2024-07-10T15:21:02Z

Updated PR to parent 55. Not merging yet to not stir things with polyglot-ruby, let's see is their issue fixed for start, and then will continue on this path.

cstamas · 2024-07-11T19:21:57Z

@lefou can you remind me... AFAIR, you insist on Java 8 build time... Due that we are stuck on Takari Lifecycle 2.0.x (as 2.1.x lifts build time requirement to Java 11, but still can produce Java 8 bytecode).

Latest Takari Lifecycle brings among other things updated GPG signing, so this "hack" by involving maven-gpg-plugin would become unnecessary as well. Just to be clear, build time we could use Java 11+ (like a matrix of 11, 17, 21, without 8 as today), but the artifacts produced would still be Java 8 bytecode. Simply put "build time Java requirement" would be raised to Java 11.

So, do you still insist on Java 8 build time?

lefou · 2024-07-15T16:41:12Z

@lefou can you remind me... AFAIR, you insist on Java 8 build time... Due that we are stuck on Takari Lifecycle 2.0.x (as 2.1.x lifts build time requirement to Java 11, but still can produce Java 8 bytecode).

Most discussion happened in #257.

In summary. I'm not against dropping Java 8 build time. But since the Scala dialect supports Java 8 runtime, we also should maintain a usable integration test suite. IIUC, if we drop Java 8 build time, we can no longer run Java 8 integration tests unless we add some toolchain setup, which isn't trivial and beyond my maintenance resources.

Latest Takari Lifecycle brings among other things updated GPG signing, so this "hack" by involving maven-gpg-plugin would become unnecessary as well. Just to be clear, build time we could use Java 11+ (like a matrix of 11, 17, 21, without 8 as today), but the artifacts produced would still be Java 8 bytecode. Simply put "build time Java requirement" would be raised to Java 11.

So, do you still insist on Java 8 build time?

No.

So, if we drop Java 8 build time support, we better also drop Java 8 runtime support for the Scala dialect due to a missing test setup. Or someone contributes an integration test setup for Java 8.

Supporting an untested product is pure evil. There is no way to reproduce issues anymore.

cstamas self-assigned this May 23, 2024

lefou reviewed May 23, 2024

View reviewed changes

cstamas force-pushed the takari-pom-54 branch from 1cd0919 to 5fdb6d7 Compare May 23, 2024 20:01

lefou reviewed May 23, 2024

View reviewed changes

lefou approved these changes May 23, 2024

View reviewed changes

cstamas force-pushed the takari-pom-54 branch from 6ce92fa to 006909c Compare May 24, 2024 09:41

cstamas added 2 commits July 10, 2024 17:15

Merge remote-tracking branch 'origin/master' into takari-pom-54

0a07390

Got for 55

433621d

cstamas changed the title ~~Bump io.takari:takari from 51 to 54~~ Bump io.takari:takari from 51 to 55 Jul 10, 2024

Fix for kt

6d035d0

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bump io.takari:takari from 51 to 55 #303

Bump io.takari:takari from 51 to 55 #303

cstamas commented May 23, 2024 •

edited

Loading

cstamas commented May 23, 2024

lefou commented May 23, 2024

cstamas commented May 23, 2024

lefou commented May 23, 2024

lefou left a comment

cstamas commented May 23, 2024

cstamas commented May 23, 2024 •

edited

Loading

lefou commented May 23, 2024

cstamas commented May 23, 2024

lefou commented May 23, 2024 •

edited

Loading

cstamas commented May 23, 2024

lefou May 23, 2024

cstamas May 23, 2024

lefou May 23, 2024

cstamas May 23, 2024

lefou May 23, 2024 •

edited

Loading

cstamas May 23, 2024

lefou left a comment

lefou May 23, 2024

cstamas May 23, 2024

lefou May 23, 2024

headius commented May 23, 2024

cstamas commented May 24, 2024 •

edited

Loading

cstamas commented Jul 10, 2024

cstamas commented Jul 11, 2024 •

edited

Loading

lefou commented Jul 15, 2024 •

edited

Loading

	# Newer versions won't work with Java 8!
	version = "3.7.15"
	version = "3.8.1"

Bump io.takari:takari from 51 to 55 #303

Are you sure you want to change the base?

Bump io.takari:takari from 51 to 55 #303

Conversation

cstamas commented May 23, 2024 • edited Loading

cstamas commented May 23, 2024

lefou commented May 23, 2024

cstamas commented May 23, 2024

lefou commented May 23, 2024

lefou left a comment

Choose a reason for hiding this comment

cstamas commented May 23, 2024

cstamas commented May 23, 2024 • edited Loading

lefou commented May 23, 2024

cstamas commented May 23, 2024

lefou commented May 23, 2024 • edited Loading

cstamas commented May 23, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lefou May 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lefou left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

headius commented May 23, 2024

cstamas commented May 24, 2024 • edited Loading

cstamas commented Jul 10, 2024

cstamas commented Jul 11, 2024 • edited Loading

lefou commented Jul 15, 2024 • edited Loading

cstamas commented May 23, 2024 •

edited

Loading

cstamas commented May 23, 2024 •

edited

Loading

lefou commented May 23, 2024 •

edited

Loading

lefou May 23, 2024 •

edited

Loading

cstamas commented May 24, 2024 •

edited

Loading

cstamas commented Jul 11, 2024 •

edited

Loading

lefou commented Jul 15, 2024 •

edited

Loading