Skip to content

[Improvement][Alert]Alert code clean#9129

Closed
huagetai wants to merge 4 commits intoapache:devfrom
huagetai:dolphinscheduler-alert
Closed

[Improvement][Alert]Alert code clean#9129
huagetai wants to merge 4 commits intoapache:devfrom
huagetai:dolphinscheduler-alert

Conversation

@huagetai
Copy link
Contributor

Purpose of the pull request

Alert code cleanup.

Brief change log

  • “ShowType.java” moved from org.apache.dolphinscheduler.alert.api to org.apache.dolphinscheduler.plugin.alert.email
  • Adjusting AlertServer only depends on AlertPluginManager、AlertRequestProcessor、AlertSender、AlertConfig,not Dao(dolphinscheduler-alert-server)

Verify this pull request

This pull request is code cleanup without any test coverage.

@songjianet songjianet requested review from caishunfeng and kezhenxu94 and removed request for kezhenxu94 March 23, 2022 08:56
@caishunfeng
Copy link
Contributor

@huagetai please check the CI test.

@huagetai huagetai marked this pull request as draft March 28, 2022 02:56
@huagetai huagetai marked this pull request as ready for review March 28, 2022 02:58
@huagetai huagetai marked this pull request as draft March 28, 2022 03:15
@huagetai huagetai marked this pull request as ready for review March 28, 2022 03:27
@caishunfeng
Copy link
Contributor

PTAL @Tianqi-Dotes

@huagetai huagetai marked this pull request as draft March 28, 2022 03:39
@codecov-commenter
Copy link

Codecov Report

Merging #9129 (0c0f81b) into dev (7022b18) will decrease coverage by 0.06%.
The diff coverage is 27.75%.

❗ Current head 0c0f81b differs from pull request most recent head 2c62896. Consider uploading reports for the commit 2c62896 to get more accurate results

@@             Coverage Diff              @@
##                dev    #9129      +/-   ##
============================================
- Coverage     40.29%   40.23%   -0.07%     
- Complexity     4366     4380      +14     
============================================
  Files           808      820      +12     
  Lines         32547    32711     +164     
  Branches       3646     3640       -6     
============================================
+ Hits          13114    13160      +46     
- Misses        18207    18319     +112     
- Partials       1226     1232       +6     
Impacted Files Coverage Δ
...r/plugin/alert/email/EmailAlertChannelFactory.java 98.63% <ø> (ø)
...eduler/plugin/alert/email/MailParamsConstants.java 0.00% <ø> (ø)
...olphinscheduler/plugin/alert/email/MailSender.java 73.42% <ø> (+0.69%) ⬆️
.../dolphinscheduler/plugin/alert/email/ShowType.java 90.90% <ø> (ø)
...ler/plugin/alert/email/template/AlertTemplate.java 0.00% <ø> (ø)
...ugin/alert/email/template/DefaultHTMLTemplate.java 82.22% <ø> (ø)
...cheduler/plugin/alert/telegram/TelegramSender.java 53.33% <0.00%> (-1.22%) ⬇️
...plugin/alert/wechat/WeChatAlertChannelFactory.java 97.82% <ø> (ø)
...hinscheduler/plugin/alert/wechat/WeChatSender.java 45.73% <ø> (ø)
...che/dolphinscheduler/alert/AlertPluginManager.java 90.90% <0.00%> (-2.85%) ⬇️
... and 61 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7022b18...2c62896. Read the comment docs.

@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

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

76.9% 76.9% Coverage
46.4% 46.4% Duplication

@Tianqi-Dotes Tianqi-Dotes self-requested a review April 1, 2022 04:02
@caishunfeng
Copy link
Contributor

Is this pr ready for review? It's draft now.

@caishunfeng
Copy link
Contributor

Hi @huagetai This part of the code has been modified in the dev, please check it.

@kezhenxu94
Copy link
Member

Long time no update. Closing. Please reopen if you want to continue.

@kezhenxu94 kezhenxu94 closed this May 23, 2022
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.

4 participants