You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by GitBox <gi...@apache.org> on 2020/10/12 11:14:47 UTC

[GitHub] [shardingsphere-elasticjob] TeslaCN opened a new pull request #1552: Trim suffix of lambda class name when using lambda expression implementation of ElasticJob

TeslaCN opened a new pull request #1552:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1552


   Fixes #1551 .
   
   Changes proposed in this pull request:
   - Rename SimpleJobClassNameProvider to DefaultJobClassNameProvider.
   - Trim suffix of lambda class name when using lambda expression implementation of ElasticJob.
   


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [shardingsphere-elasticjob] codecov-io commented on pull request #1552: Trim suffix of lambda class name when using lambda expression implementation of ElasticJob

Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #1552:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1552#issuecomment-707079663


   # [Codecov](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1552?src=pr&el=h1) Report
   > Merging [#1552](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1552?src=pr&el=desc) into [master](https://codecov.io/gh/apache/shardingsphere-elasticjob/commit/78d68d5b71b1b71046b23f30c8d9de1e1d87ddb7?el=desc) will **decrease** coverage by `0.06%`.
   > The diff coverage is `57.14%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1552/graphs/tree.svg?width=650&height=150&src=pr&token=8ZMVc4Yo4Z)](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1552?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #1552      +/-   ##
   ============================================
   - Coverage     85.27%   85.21%   -0.07%     
     Complexity      101      101              
   ============================================
     Files           247      247              
     Lines          5670     5674       +4     
     Branches        886      888       +2     
   ============================================
     Hits           4835     4835              
   - Misses          525      526       +1     
   - Partials        310      313       +3     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1552?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...te/internal/setup/DefaultJobClassNameProvider.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1552/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1saXRlL2VsYXN0aWNqb2ItbGl0ZS1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9lbGFzdGljam9iL2xpdGUvaW50ZXJuYWwvc2V0dXAvRGVmYXVsdEpvYkNsYXNzTmFtZVByb3ZpZGVyLmphdmE=) | `50.00% <50.00%> (ø)` | `1.00 <1.00> (?)` | |
   | [...te/internal/setup/JobClassNameProviderFactory.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1552/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1saXRlL2VsYXN0aWNqb2ItbGl0ZS1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9lbGFzdGljam9iL2xpdGUvaW50ZXJuYWwvc2V0dXAvSm9iQ2xhc3NOYW1lUHJvdmlkZXJGYWN0b3J5LmphdmE=) | `57.14% <100.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...ticjob/lite/internal/snapshot/SnapshotService.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1552/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1saXRlL2VsYXN0aWNqb2ItbGl0ZS1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9lbGFzdGljam9iL2xpdGUvaW50ZXJuYWwvc25hcHNob3QvU25hcHNob3RTZXJ2aWNlLmphdmE=) | `80.95% <0.00%> (-1.59%)` | `1.00% <0.00%> (ø%)` | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1552?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1552?src=pr&el=footer). Last update [78d68d5...48512cf](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1552?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [shardingsphere-elasticjob] Technoboy- commented on a change in pull request #1552: Trim suffix of lambda class name when using lambda expression implementation of ElasticJob

Posted by GitBox <gi...@apache.org>.
Technoboy- commented on a change in pull request #1552:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1552#discussion_r503245653



##########
File path: elasticjob-lite/elasticjob-lite-core/src/main/java/org/apache/shardingsphere/elasticjob/lite/internal/setup/DefaultJobClassNameProvider.java
##########
@@ -22,10 +22,20 @@
 /**
  * Simple job class name provider.
  */
-public final class SimpleJobClassNameProvider implements JobClassNameProvider {
+public final class DefaultJobClassNameProvider implements JobClassNameProvider {
     
     @Override
     public String getJobClassName(final ElasticJob elasticJob) {
-        return elasticJob.getClass().getName();
+        Class<? extends ElasticJob> elasticJobClass = elasticJob.getClass();
+        String elasticJobClassName = elasticJobClass.getName();
+        return isLambdaClass(elasticJobClass) ? trimLambdaClassSuffix(elasticJobClassName) : elasticJobClassName;
+    }
+    
+    private boolean isLambdaClass(final Class<? extends ElasticJob> elasticJobClass) {

Review comment:
       Add test case to cover this.




----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [shardingsphere-elasticjob] TeslaCN commented on a change in pull request #1552: Trim suffix of lambda class name when using lambda expression implementation of ElasticJob

Posted by GitBox <gi...@apache.org>.
TeslaCN commented on a change in pull request #1552:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1552#discussion_r503248310



##########
File path: elasticjob-lite/elasticjob-lite-core/src/main/java/org/apache/shardingsphere/elasticjob/lite/internal/setup/DefaultJobClassNameProvider.java
##########
@@ -22,10 +22,20 @@
 /**
  * Simple job class name provider.
  */
-public final class SimpleJobClassNameProvider implements JobClassNameProvider {
+public final class DefaultJobClassNameProvider implements JobClassNameProvider {
     
     @Override
     public String getJobClassName(final ElasticJob elasticJob) {
-        return elasticJob.getClass().getName();
+        Class<? extends ElasticJob> elasticJobClass = elasticJob.getClass();
+        String elasticJobClassName = elasticJobClass.getName();
+        return isLambdaClass(elasticJobClass) ? trimLambdaClassSuffix(elasticJobClassName) : elasticJobClassName;
+    }
+    
+    private boolean isLambdaClass(final Class<? extends ElasticJob> elasticJobClass) {

Review comment:
       Testcase will be committed in a new PR.




----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [shardingsphere-elasticjob] terrymanu merged pull request #1552: Trim suffix of lambda class name when using lambda expression implementation of ElasticJob

Posted by GitBox <gi...@apache.org>.
terrymanu merged pull request #1552:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1552


   


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [shardingsphere-elasticjob] Technoboy- commented on a change in pull request #1552: Trim suffix of lambda class name when using lambda expression implementation of ElasticJob

Posted by GitBox <gi...@apache.org>.
Technoboy- commented on a change in pull request #1552:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1552#discussion_r503244898



##########
File path: elasticjob-lite/elasticjob-lite-core/src/main/java/org/apache/shardingsphere/elasticjob/lite/internal/setup/DefaultJobClassNameProvider.java
##########
@@ -22,10 +22,20 @@
 /**
  * Simple job class name provider.
  */
-public final class SimpleJobClassNameProvider implements JobClassNameProvider {
+public final class DefaultJobClassNameProvider implements JobClassNameProvider {
     
     @Override
     public String getJobClassName(final ElasticJob elasticJob) {

Review comment:
       It's better to add test case to cover this.




----------------------------------------------------------------
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:
users@infra.apache.org