Skip to content

Conversation

@Flowdalic
Copy link
Contributor

This adds a few simple and short tests for the Scala 3 macros. Especially for the central deconstructMessageFormat() macro, which was previously broken and fixed with 66ac4a7 ("Fixed Scala 3 macros (#26)").

@Flowdalic
Copy link
Contributor Author

@vy In #26 @pjfanning asked for tests for the macros. Here are the tests that I used to debug and fix the macros. Nothing fancy, but I figured I try to contribute before those tests are lost.

@vy
Copy link
Member

vy commented Nov 6, 2023

@Flowdalic, thanks so much! Would you mind fixing the compilation errors, please?

@vy vy self-assigned this Nov 6, 2023
@vy vy added the enhancement label Nov 6, 2023
@Flowdalic
Copy link
Contributor Author

@Flowdalic, thanks so much! Would you mind fixing the compilation errors, please?

Sure. Couldn't figure out that I need ./mvnw test -pl log4j-api-scala_3 to run just the Scala 3 tests. Should compile now :)

@vy
Copy link
Member

vy commented Nov 6, 2023

@Flowdalic, I also need your commits to be signed to be able to merge this PR.

I appreciate it if you can sign the commits. This will give you the full credit in the git log. Otherwise, I can apply your changes myself, I will appear as the committer – though I will link to this issue, where those who are curious can trace back to you.

@Flowdalic
Copy link
Contributor Author

I appreciate it if you can sign the commits.

Done.

@vy
Copy link
Member

vy commented Nov 6, 2023

@Flowdalic, I think you did a git commit -s. You should rather have done git commit -S. Could you try that and -f push again?

This adds a few simple and short tests for the Scala 3
macros. Especially for the central deconstructMessageFormat() macro,
which was previously broken and fixed with 66ac4a7 ("Fixed Scala
3 macros (apache#26)").

Signed-off-by: Florian Schmaus <flo@geekplace.eu>
@Flowdalic
Copy link
Contributor Author

@Flowdalic, I think you did a git commit -s. You should rather have done git commit -S. Could you try that and -f push again?

Oh, you really asked for an OpenPGP signed commit. Not just a signed off trailer. Done.

@vy vy merged commit 91396bf into apache:main Nov 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants