-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Record support #840
base: master
Are you sure you want to change the base?
Record support #840
Conversation
Fixes square#829 Signed-off-by: liach <[email protected]>
Hi @JakeWharton - can you possibly let the community know the plans for JavaPoet and support for post Java 11 features? Will this PR or something similar be committed to the codebase soon? Thank you. |
I no longer participate in the day-to-day of this repository or have the bandwidth to review changes with such large API surface. Hopefully @Egorand or someone will pick it up. |
If Square isn't going to continue with Javapoet maybe we can fork it and continue supporting it. I'd be happy to host/republish it. Let's see what @Egorand says. |
You are always free to do so, but don't mistake my lack of involvement which has been consistent for the last 5 years as any larger indicator. I was mentioned personally therefore I commented on my personal involvement. |
Thanks @JakeWharton |
Any updates on this? This is one of the exciting features of Java 17 since it's now been released. |
@Egorand Mind review this, or is javapoet of low priority right now? |
Apologies for not replying sooner and thanks for your contribution! Unfortunately I lost context on this project and Java in general a while ago and can't review this efficiently at the moment. I'll try to find someone who can, or hopefully find some time in the near term to catch up. |
I reviewed the code and I think it is good to go. It doesn't break the existing API nor logic and introduces Record support. I'm also in for forking this project, if there is no interest or time from Square anymore to maintain this project. |
Is there any good alternative to JavaPoet that supports generating records? |
If we do fork, what needs to be done? I have a Maven Group ID prefix we can use and I can already publish to Maven Central with it. I think we'd have to change the project's group ID at minimum. Do we have to change the license? Copyright? Package names? Anyone know? |
You should not change the license or copyright, that would be illegal. |
The difference here is that I would like to re-publish this so others can use/contribute. That's allow under AL2 right? |
Changing the license isn't illegal but shouldn't be necessary. I wonder if the package name should be changed though. Publishing to Maven Central is more trustworthy than jitpack (which is denylisted by many organizations). In my mind, the main question is whether someone would be willing/able to maintain a fork in the long run. |
You can only modify the license, if you were the original author / copyright holder. Probably in javapoet case, it would mean if all the contributor agree on a license change. But I don't think license change is needed, AL2 is a really permissive license, you can freely modify, publish,etc. |
Fyi I have a fork including this content, available over github packages https://github.com/liachmodded/javapoet |
I don't think this is true (https://en.wikipedia.org/wiki/Apache_License#Licensing_conditions). But as you said, there's no need to change the license. |
Can you add a test with a secondary constructor? Something like record Person(String name) {
Person(int number) {
this.name = Integer.toString(number);
}
} |
yeah, will do later today |
Looks good otherwise. I'll have another read of the JLS just to make sure we're covering our bases. |
Signed-off-by: liach <[email protected]>
Done. CI results available at https://github.com/liachmodded/javapoet/runs/4255899160 (The java 8 one fails because setup java action cannot find java 8 by |
I thought about this over the weekend more. I'm not sure it makes sense to put the record parameters directly on the type. What do you think about instead changing |
Such a setup may be feasible, but compared to the current impl, there are a few behaviors to be defined:
|
How can we get this unblocked? Do the maintainers intend to continue this project? |
I was looking for this as well... |
Update from SquareApologies for radio silence on this. We're busy! I'm talking to our internal copyright & IP legal team to hand off the project. There's logistics here to figure out how & who. But the plan is to hand-off JavaPoet to folks that can give it the attention it deserves. |
@swankjesse Thx for the clarification. |
Another month has passed. Are they any news on handing off the project? |
Handing off the project is orthogonal to this PR. And this is not a good place to discuss it. There's a discussions tab. Please use that from now on for project-wide discussion topics. As to this PR, I expressed concern with the existing shape of the API design so it will not be merged as-is. Either the changes need made to this PR or a new PR needs opened. We are not working on this project so we no one from our side will be making the changes required to land this support. That is unfortunate, but placing this project into the open is not a promise that we would work on it forever. It exists in open source for you to use, or not use, free of charge. If you are blocked and need this API, forking the project and adding support through this PR's diff or your own design is not that hard to do. You will have to maintain that fork going forward, but doing so is orders of magnitude more trivial than having to write an entire Java code generation API in the first place. There are surely other libraries or template engines that you could use as well. Please keep future discussions on this PR about the design of the API in the diff. |
@JakeWharton: There is not a discussions tab turned on for this repo. |
Ah, thanks for the heads up! The tab was enabled but apparently I had to do a one-time setup thing (which I do not recall doing on other repos). |
DIscussion: #866 |
- square#934 - square#925 - square#913 - square#840 - more support for records - slight fixes
Is there anyone who can tell what's the state of this PR? |
From square/javapoet#840 User: @liach
Fixes #829
A short summary:
Added record support to javapoet as it has become a stable feature in modern java. Turns out the support is quite easy given the robust existing framework in javapoet.
mvn clean verify
ends with 17 file-ending errors (presumably because of crlf/lf problems), which I cannot fix locally; all tests pass. CLA signed as well.It's my pleasure to have you reviewing my code.
P.S. for more information about java records, feel free to check out the related java language specification section and look at a jdk example and its javadocs