chengshiwen commented on a change in pull request #4586:
URL:
https://github.com/apache/incubator-dolphinscheduler/pull/4586#discussion_r578908138
##########
File path:
dolphinscheduler-alert/src/test/java/org/apache/dolphinscheduler/alert/plugin/AlertPluginManagerTest.java
##########
@@ -60,6 +62,6 @@ public void testLoadPlugins() throws Exception {
throw new RuntimeException("load Alert Plugin Failed !", e);
}
-
Assert.assertNotNull(alertPluginManager.getAlertChannelFactoryMap().get("email
alert"));
+
Assert.assertNotNull(alertPluginManager.getAlertChannelFactoryMap().get("Email"));
Review comment:
Avoid the magic number `Email`, and it's better to use a **Constant**
##########
File path:
dolphinscheduler-alert/src/test/java/org/apache/dolphinscheduler/alert/AlertServerTest.java
##########
@@ -18,8 +18,8 @@
package org.apache.dolphinscheduler.alert;
import org.apache.dolphinscheduler.alert.plugin.AlertPluginManager;
-import org.apache.dolphinscheduler.alert.plugin.DolphinPluginLoader;
-import org.apache.dolphinscheduler.alert.plugin.DolphinPluginManagerConfig;
+import org.apache.dolphinscheduler.common.plugin.DolphinPluginLoader;
+import org.apache.dolphinscheduler.common.plugin.DolphinPluginManagerConfig;
Review comment:
import order problem
##########
File path:
dolphinscheduler-alert/src/test/java/org/apache/dolphinscheduler/alert/plugin/DolphinPluginLoaderTest.java
##########
@@ -55,6 +48,6 @@ public void testLoadPlugins() throws Exception {
throw new RuntimeException("load Alert Plugin Failed !", e);
}
-
Assert.assertNotNull(alertPluginManager.getAlertChannelFactoryMap().get("email
alert"));
+
Assert.assertNotNull(alertPluginManager.getAlertChannelFactoryMap().get("Email"));
Review comment:
Avoid the magic number `Email`
##########
File path:
dolphinscheduler-alert/src/main/java/org/apache/dolphinscheduler/alert/AlertServer.java
##########
@@ -20,8 +20,8 @@
import static org.apache.dolphinscheduler.common.Constants.ALERT_RPC_PORT;
import org.apache.dolphinscheduler.alert.plugin.AlertPluginManager;
-import org.apache.dolphinscheduler.alert.plugin.DolphinPluginLoader;
-import org.apache.dolphinscheduler.alert.plugin.DolphinPluginManagerConfig;
+import org.apache.dolphinscheduler.common.plugin.DolphinPluginLoader;
+import org.apache.dolphinscheduler.common.plugin.DolphinPluginManagerConfig;
Review comment:
These lines will resulting **import order** problems and **check style
failure**
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
