-
Notifications
You must be signed in to change notification settings - Fork 409
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
Fixing Mail deserialization issue. #225
Conversation
@thinkingserious have you ever considered adopting Project Lombok library in order to widely reduce boilerplate coding? |
Hi @TBuc, This is the first I've heard of it, thanks for the heads up! Would that be something that makes this PR easier? |
Hi @thinkingserious, actually yes, it would make the PR easier as, in example: PR for file BccSettings.java would reduce to just two lines of code:
PR for file Mail.java would require two lines but a little more effort since I can see you are not including all its parameters in the equals() and compareTo() methods. You will then need to create a list of the parameters to be excluded (I will just list part of them for this example):
That said, I embrace (and love) lombok, but you should take a minute to decide whether to adopt it or not. As any library, it has its wild sides too (even thought I did not find any in this specific annotation yet), and a build goal should be added for providing delomboking of classes, also in order to be able to check the code it produces for you. |
Hi @TBuc, Let's revisit this after we get this one done. Your feedback there is greatly appreciated as well :) Thanks! With Best Regards, Elmer |
I cleaned up the merge conflicts and pushed in all the recent changes |
Hi @sccalabr, Could you please resolve these merge conflicts? Sorry about the hassle :( With Best Regards, Elmer |
Hello @sccalabr, |
#189