Skip to content

[11130][refactor] refactor alert-api with lombok#11133

Merged
kezhenxu94 merged 5 commits intoapache:devfrom
MichaelDeSteven:refactor_alert_with_lombok
Jul 28, 2022
Merged

[11130][refactor] refactor alert-api with lombok#11133
kezhenxu94 merged 5 commits intoapache:devfrom
MichaelDeSteven:refactor_alert_with_lombok

Conversation

@MichaelDeSteven
Copy link
Contributor

Purpose of the pull request

see #11130

Brief change log

refactor alert-api with lombok

Verify this pull request

This pull request is code cleanup without any test coverage.

Copy link
Member

@kezhenxu94 kezhenxu94 left a comment

Choose a reason for hiding this comment

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

Use @Data to replace @Getter, @Setter, @ToString

@MichaelDeSteven
Copy link
Contributor Author

Use @Data to replace @Getter, @Setter, @ToString

Is that means we need to replace all class with @Data or just AlertData AlertInfo AlertResult.
I saw #11074 say that would better to avoid use @Data.

@MichaelDeSteven MichaelDeSteven changed the title refactor: refactor alert-api with lombok [11130][refactor] refactor alert-api with lombok Jul 26, 2022
@EricGao888
Copy link
Member

Use @Data to replace @Getter, @Setter, @ToString

Hi @kezhenxu94, when I was working on Zeppelin task plugin, I used @Data to refactor ZeppelinParameters at first and caused sonar to fail because of a significant decrease in UT coverage. After checking some materials, I found @Data generates some code we do not need here. After replacing it with @Getter @Setter and ToString, the UT coverage recovered. So there might be a trade-off between code briefness and UT coverage.
Some related materials:

WDYT?

@kezhenxu94
Copy link
Member

Use @Data to replace @Getter, @Setter, @ToString

Hi @kezhenxu94, when I was working on Zeppelin task plugin, I used @Data to refactor ZeppelinParameters at first and caused sonar to fail because of a significant decrease in UT coverage. After checking some materials, I found @Data generates some code we do not need here. After replacing it with @Getter @Setter and ToString, the UT coverage recovered. So there might be a trade-off between code briefness and UT coverage.

Some related materials:

WDYT?

We should be able to exclude generated codes from Sonar

@SbloodyS SbloodyS added the improvement make more easy to user or prompt friendly label Jul 26, 2022
@SbloodyS SbloodyS added this to the 3.1.0-alpha milestone Jul 26, 2022
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

56.2% 56.2% Coverage
0.0% 0.0% Duplication

Copy link
Member

@EricGao888 EricGao888 left a comment

Choose a reason for hiding this comment

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

Good job! LGTM

Copy link
Contributor

@zhuxt2015 zhuxt2015 left a comment

Choose a reason for hiding this comment

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

LGTM

@kezhenxu94 kezhenxu94 merged commit a52d1b4 into apache:dev Jul 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend improvement make more easy to user or prompt friendly

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants