Skip to content

[Improvement-18151] Simplify the code with lombok annotations#18174

Open
njnu-seafish wants to merge 15 commits intoapache:devfrom
njnu-seafish:Improvement-18151-suyc
Open

[Improvement-18151] Simplify the code with lombok annotations#18174
njnu-seafish wants to merge 15 commits intoapache:devfrom
njnu-seafish:Improvement-18151-suyc

Conversation

@njnu-seafish
Copy link
Copy Markdown
Contributor

Was this PR generated or assisted by AI?

YES, First, use AI to identify redundant code, then manually verify and simplify it.

Purpose of the pull request

part of #18151

Brief change log

Simplify the code with lombok annotations

Verify this pull request

This pull request is code cleanup without any test coverage.

(or)

This pull request is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(or)

Pull Request Notice

Pull Request Notice

If your pull request contains incompatible change, you should also add it to docs/docs/en/guide/upgrade/incompatible.md

Copy link
Copy Markdown
Member

@ruanwenjun ruanwenjun left a comment

Choose a reason for hiding this comment

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

Please do not modify non-POJO objects


@ToString
@Slf4j
@Data
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not all field should exposed to out layer, e.g latch, don't change the code you are not clear.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ok

Comment on lines -206 to -211
public void setDepList(List<Long> depList) {
if (depList != null) {
this.depList = depList;
this.preTasks = JSONUtils.toJsonString(depList);
}
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we remove this?

Copy link
Copy Markdown
Contributor Author

@njnu-seafish njnu-seafish Apr 17, 2026

Choose a reason for hiding this comment

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

The project compiles successfully, and this method is unused.
企业微信截图_17764180363744

Comment on lines +28 to +29
@Data
@Accessors(chain = true)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
@Data
@Accessors(chain = true)
@Accessors(chain = true)

Why add this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Why add this?

Here is the code before refactoring:

public ParamsProps setPlaceholder(String placeholder) {
    this.placeholder = placeholder;
    return this;
}

Yes, this is not used.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Why add this?

ok, revert those changes

@Accessors(chain = true)
public class ParamsProps {

@JsonProperty("placeholder")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why add this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Why add this?

Here is the code before refactoring:

public ParamsProps setPlaceholder(String placeholder) {
this.placeholder = placeholder;
return this;
}
Yes, this is not used.

企业微信截图_17764183474087

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Why add this?

ok, revert those changes

@njnu-seafish njnu-seafish requested a review from ruanwenjun April 17, 2026 09:32
@ruanwenjun ruanwenjun force-pushed the Improvement-18151-suyc branch from 604ccf3 to ab497fe Compare April 28, 2026 01:45
import lombok.Data;
import lombok.EqualsAndHashCode;

@EqualsAndHashCode(callSuper = true)
import lombok.Data;
import lombok.EqualsAndHashCode;

@EqualsAndHashCode(callSuper = true)
import lombok.Data;
import lombok.EqualsAndHashCode;

@EqualsAndHashCode(callSuper = true)
import lombok.Data;
import lombok.EqualsAndHashCode;

@EqualsAndHashCode(callSuper = true)
import lombok.Data;
import lombok.EqualsAndHashCode;

@EqualsAndHashCode(callSuper = true)
@njnu-seafish
Copy link
Copy Markdown
Contributor Author

Reverted some unnecessary changes. Please take another look. Thanks! @ruanwenjun @SbloodyS

@SbloodyS
Copy link
Copy Markdown
Member

SbloodyS commented May 7, 2026

CI failed. @njnu-seafish

@njnu-seafish
Copy link
Copy Markdown
Contributor Author

njnu-seafish commented May 8, 2026

CI failed. @njnu-seafish

It's strange, here is the error message. It compiles and runs fine on my local machine. Also, the changes I made shouldn't cause this error.

[INFO] --- maven-compiler-plugin:3.3:testCompile (default-testCompile) @ dolphinscheduler-dao ---
[INFO] Changes detected - recompiling the module!
[INFO] Compiling 51 source files to /home/runner/work/dolphinscheduler/dolphinscheduler/dolphinscheduler-dao/target/test-classes
[INFO] -------------------------------------------------------------
Error: COMPILATION ERROR :
[INFO] -------------------------------------------------------------
Error: /home/runner/work/dolphinscheduler/dolphinscheduler/dolphinscheduler-dao/src/test/java/org/apache/dolphinscheduler/dao/repository/impl/TaskGroupQueueDaoImplTest.java:[30,39] package org.apache.commons.collections4 does not exist
Error: /home/runner/work/dolphinscheduler/dolphinscheduler/dolphinscheduler-dao/src/test/java/org/apache/dolphinscheduler/dao/repository/impl/TaskGroupQueueDaoImplTest.java:[31,32] package org.apache.commons.lang3 does not exist
Error: /home/runner/work/dolphinscheduler/dolphinscheduler/dolphinscheduler-dao/src/test/java/org/apache/dolphinscheduler/dao/repository/impl/WorkflowInstanceDaoImplTest.java:[20,46] package org.apache.commons.collections4 does not exist
Error: /home/runner/work/dolphinscheduler/dolphinscheduler/dolphinscheduler-dao/src/test/java/org/apache/dolphinscheduler/dao/repository/impl/WorkflowInstanceDaoImplTest.java:[20,1] static import only from classes and interfaces
Error: /home/runner/work/dolphinscheduler/dolphinscheduler/dolphinscheduler-dao/src/test/java/org/apache/dolphinscheduler/dao/mapper/AlertMapperTest.java:[26,39] package org.apache.commons.codec.digest does not exist
Error: /home/runner/work/dolphinscheduler/dolphinscheduler/dolphinscheduler-dao/src/test/java/org/apache/dolphinscheduler/dao/repository/impl/CommandDaoImplTest.java:[33,32] package org.apache.commons.lang3 does not exist
[INFO] 6 errors
[INFO] -------------------------------------------------------------
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 6.904 s
[INFO] Finished at: 2026-05-07T02:24:50Z
[INFO] ------------------------------------------------------------------------

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented May 9, 2026

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 60%)

See analysis details on SonarQube Cloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants