Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fix for #2693] Adding a new accessors flag - javaBeansSpecCapitalization #2996

Merged
merged 2 commits into from Oct 18, 2021

Conversation

YonatanSherwin
Copy link

Adding a new accessors flag, which will allow to follow Java Bean Spec capitalization rules (e.g aField -> getaField())

@rzwitserloot
Copy link
Collaborator

Gracias, @YonatanSherwin I'm looking it over now. Can you add one more commit to this where you add your name to the AUTHORS file? Please note that this act (as explained at the bottom of that file) means you consent that your work will fall under lombok's copyright rules.

A few updates I'm currently making:

  • The config key is becoming an enum, with valid values: "basic" and "beanspec". I'm not foreseeing more than 2, but attempting to attach 'true' and 'false' to 2 different strategies is a bit unsightly, I think.
  • Your code didn't handle builders, which also generates method names (such as clearAddresses) that need to adhere to the chosen configuration option.
  • The option to set this option directly as param on the @Accessors annotation is going to go away. Our general rule is: If we can't imagine serious scenarios where one would wish to have different applications of a setting within a single package, we don't allow it to be an on-annotation setting, because we acknowledge that we have limited room for such settings (we've messed up somewhere if a lombok annotation has a menu of options as long as my leg, especially if most of them seem like a mistake if you set them on the annotation instead of as a project-wide config). If one day we realize that it is common enough to offer it, we can always add it; removing it is much harder as that breaks backwards compatibility.

Said differently, if it really feels like the annotation parameter should get the following line in its documentation: "... but you probably dont want to use this and instead set a config key for the entire project", we don't add it :)

@rzwitserloot rzwitserloot self-requested a review October 18, 2021 13:45
@rzwitserloot rzwitserloot self-assigned this Oct 18, 2021
@rzwitserloot rzwitserloot added this to the next-version milestone Oct 18, 2021
@rzwitserloot rzwitserloot removed their request for review October 18, 2021 13:47
@rzwitserloot
Copy link
Collaborator

I've finished the above updates. Next step is adding some docs, updating changelog, integrate your AUTHORS update, and push an edge release.

@rzwitserloot
Copy link
Collaborator

Docs and changelog done too. Whenever you're ready, @YonatanSherwin :)

@YonatanSherwin
Copy link
Author

Thanks for the great comments @rzwitserloot !
Where i can see the changes you've did?

@rzwitserloot rzwitserloot merged commit 1ae87ba into projectlombok:master Oct 18, 2021
@rzwitserloot
Copy link
Collaborator

Commit 13d84b1 for the docs, and commit 13d84b1 for the updates.

@rzwitserloot
Copy link
Collaborator

The current edge includes this update (the changelog listed on the edge download page is wrong, it's in there):

https://projectlombok.org/download-edge

@YonatanSherwin
Copy link
Author

Thanks @rzwitserloot !
BTW i've created a new PR for fixing two tests failures:
#2998

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change lombok's property capitalization code. Field xName -> getxName().
2 participants