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

Delombok #9

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Delombok #9

wants to merge 5 commits into from

Conversation

stokito
Copy link
Contributor

@stokito stokito commented Nov 3, 2024

The Lombok on a such small codebase is not so helpful but makes build process more complicated.
The code reporting tools like Checkstyle and Spotbugs seems to report lines incorrectly.
I upgraded the Checkstyle to latest version that works on Java 8. Also the Spotbugs may be upgraded but... it then reports even more warnings that breaks a build.

Copy link
Contributor

@GregDThomas GregDThomas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A number of unrelated commits seem to have crept in with this one.

Also, by removing the @DaTa tag some of the classes have lost toString, dashcode and equals. In hindsight, at least one of those should have not have had the new password in the toString() (@ToString.Exclude), but a toString() method would be useful when checking logs.

@stokito
Copy link
Contributor Author

stokito commented Nov 3, 2024

@GregDThomas Sorry, I forgot to mention: I made the changes on top of the previous PR. SO in fact you may check only the last two commits.

@stokito
Copy link
Contributor Author

stokito commented Nov 3, 2024

I added toString() to all DTOs with all fields except of passwords

@GregDThomas
Copy link
Contributor

OK - but the pipeline is failing with check style issues

@stokito
Copy link
Contributor Author

stokito commented Nov 3, 2024

fixed, force pushed, build passed

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.

2 participants