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

[Improvement][Tools] Remove Mapper usage in upgrade tools #15073

Merged
merged 1 commit into from
Oct 25, 2023

Conversation

ruanwenjun
Copy link
Member

@ruanwenjun ruanwenjun commented Oct 24, 2023

Purpose of the pull request

close #15069

Brief change log

  • Remove mapper usage in upgrade tools
  • Move database dialect into dao plugin module
  • Move mybatis configuration in application.yaml

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)

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

@ruanwenjun ruanwenjun added the improvement make more easy to user or prompt friendly label Oct 24, 2023
@mergeable mergeable bot removed the improvement make more easy to user or prompt friendly label Oct 24, 2023
@ruanwenjun ruanwenjun added improvement make more easy to user or prompt friendly backend labels Oct 24, 2023
@github-actions github-actions bot added improvement make more easy to user or prompt friendly backend labels Oct 24, 2023
@ruanwenjun ruanwenjun force-pushed the dev_wenjun_addPGV16IT branch 10 times, most recently from 0e86040 to 8be4282 Compare October 25, 2023 13:06
@ruanwenjun
Copy link
Member Author

image The one bug is a regex, this is old code, I don't want to change in this pr.

@@ -130,19 +130,16 @@
<dependency>
<groupId>org.testcontainers</groupId>
<artifactId>mysql</artifactId>
<scope>test</scope>
Copy link
Contributor

Choose a reason for hiding this comment

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

why remove this scope?

Copy link
Member Author

@ruanwenjun ruanwenjun Oct 25, 2023

Choose a reason for hiding this comment

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

Since the scope is managed at dolphinscheduler-bom

@caishunfeng
Copy link
Contributor

LGTM, can you add some docs for guiding on how to write an upgrade program and why we should remove mapper?

@sonarcloud
Copy link

sonarcloud bot commented Oct 25, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug C 1 Bug
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 83 Code Smells

4.6% 4.6% Coverage
2.2% 2.2% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

@ruanwenjun ruanwenjun changed the title Remove mapper usage in tools [Improvement][Tools] Remove Mapper usage in upgrade tools Oct 25, 2023
@ruanwenjun ruanwenjun merged commit a5284e4 into apache:dev Oct 25, 2023
52 of 54 checks passed
@ruanwenjun ruanwenjun deleted the dev_wenjun_addPGV16IT branch October 25, 2023 14:58
xdu-chenrj pushed a commit to xdu-chenrj/dolphinscheduler that referenced this pull request Oct 30, 2023
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 ready-to-merge refactor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Improvement][Tools] Remove Mapper usage in upgrade tools
2 participants