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

We need more code comments #2884

Closed
carryxyh opened this issue Dec 4, 2018 · 20 comments
Closed

We need more code comments #2884

carryxyh opened this issue Dec 4, 2018 · 20 comments
Labels
help wanted Everything needs help from contributors

Comments

@carryxyh
Copy link
Member

carryxyh commented Dec 4, 2018

目前我们的代码中,注释的覆盖率非常低,这对于初入dubbo的开发者是不友好的,我们应该开始着手解决这个问题。非常欢迎开发者为我们提交PR来完善注释。

At present, the coverage of comments in our code is very low, which is not friendly to developers who are new to dubbo. We should start to solve this problem. Developers are welcome to submit PRs for us to complete the comments.
:)

@carryxyh carryxyh added help wanted Everything needs help from contributors good first issue labels Dec 4, 2018
@oldratlee
Copy link
Contributor

oldratlee commented Dec 4, 2018

Attach the the coverage data of comments in code(commit id: 1f751a6 ):

$ ohcount **/src/main/; ohcount **/src/test/
Examining 855 file(s)

                          Ohloh Line Count Summary

Language          Files       Code    Comment  Comment %      Blank      Total
----------------  -----  ---------  ---------  ---------  ---------  ---------
java                770      56411      22867      28.8%      12220      91498
xmlschema             2       2563          0       0.0%         67       2630
shell                 5        240          6       2.4%         26        272
xml                   4         40         71      64.0%         25        136
bat                   1         22          1       4.3%          6         29
----------------  -----  ---------  ---------  ---------  ---------  ---------
Total               782      59276      22945      27.9%      12344      94565
Examining 746 file(s)

                          Ohloh Line Count Summary

Language          Files       Code    Comment  Comment %      Blank      Total
----------------  -----  ---------  ---------  ---------  ---------  ---------
java                631      47409      12869      21.3%      10913      71191
xml                  58        786        935      54.3%        348       2069
----------------  -----  ---------  ---------  ---------  ---------  ---------
Total               689      48195      13804      22.3%      11261      73260

@khanimteyaz
Copy link
Contributor

I am new to open source contribution world and new to dubbo also. I see this is has been tagged as 'Help wanted' and 'good first issue'. Is there any way I help one this one. Would request someone to be my mentor guide me here as well.

@ralf0131
Copy link
Contributor

ralf0131 commented Dec 4, 2018

@oldratlee Can you break it down by modules? We can start from the lowest.

@chenlushun
Copy link
Contributor

I recommend that when you write comments, you change lines in the right place so that it's easy to read. The following comments needs to slide.

/**
     * Register data, such as : provider service, consumer address, route rule, override rule and other data.
     * <p>
     * Registering is required to support the contract:<br>
     * 1. When the URL sets the check=false parameter. When the registration fails, the exception is not thrown and retried in the background. Otherwise, the exception will be thrown.<br>
     * 2. When URL sets the dynamic=false parameter, it needs to be stored persistently, otherwise, it should be deleted automatically when the registrant has an abnormal exit.<br>
     * 3. When the URL sets category=routers, it means classified storage, the default category is providers, and the data can be notified by the classified section. <br>
     * 4. When the registry is restarted, network jitter, data can not be lost, including automatically deleting data from the broken line.<br>
     * 5. Allow URLs which have the same URL but different parameters to coexist,they can't cover each other.<br>
     *
     * @param url  Registration information , is not allowed to be empty, e.g: dubbo://10.20.153.10/org.apache.dubbo.foo.BarService?version=1.0.0&application=kylin
     */
    void register(URL url);

@lovepoem
Copy link
Member

lovepoem commented Dec 4, 2018 via email

@oldratlee
Copy link
Contributor

oldratlee commented Dec 4, 2018

@ralf0131
@oldratlee Can you break it down by modules? We can start from the lowest.

comment coverage data per module sorted by product code:

module product code test code
dubbo-registry 18.2% 19.2%
dubbo-monitor 25.3% 17.1%
dubbo-config 25.8% 24.5%
dubbo-common 25.9% 23.2%
dubbo-rpc 27.4% 18.3%
dubbo-remoting 29.8% 22.4%
dubbo-container 31.5% 55.3%
dubbo-plugin 34.3% 32.6%
dubbo-filter 34.7% 32.5%
dubbo-serialization 35.2% 16.8%
dubbo-cluster 35.5% 19.4%
dubbo-compatible 40.3% 23.8%
dubbo-metrics 50.8% 19.9%
dubbo-test 51.5% -
dubbo-demo 62.7% -

PS:
If needed, I can write a script to generate above table.


Below is the quick-hack script to show coverage data per module:

$ for d in `find -mindepth 1 -maxdepth 1 -type d -name 'dubbo*'`; do  (
  echo
  echo "===================================="
  echo $d
  echo "===================================="
  cd $d
  ohcount **/src/main/; ohcount **/src/test/
)
done

......
====================================
./dubbo-cluster
====================================
Examining 67 file(s)

                          Ohloh Line Count Summary

Language          Files       Code    Comment  Comment %      Blank      Total
----------------  -----  ---------  ---------  ---------  ---------  ---------
java                 62       2544       1400      35.5%        490       4434
----------------  -----  ---------  ---------  ---------  ---------  ---------
Total                62       2544       1400      35.5%        490       4434
Examining 38 file(s)

                          Ohloh Line Count Summary

Language          Files       Code    Comment  Comment %      Blank      Total
----------------  -----  ---------  ---------  ---------  ---------  ---------
java                 34       3421        822      19.4%        706       4949
xml                   1         13         14      51.9%          2         29
----------------  -----  ---------  ---------  ---------  ---------  ---------
Total                35       3434        836      19.6%        708       4978

====================================
./dubbo-common
====================================
Examining 121 file(s)

                          Ohloh Line Count Summary

Language          Files       Code    Comment  Comment %      Blank      Total
----------------  -----  ---------  ---------  ---------  ---------  ---------
java                115      15045       5248      25.9%       2610      22903
----------------  -----  ---------  ---------  ---------  ---------  ---------
Total               115      15045       5248      25.9%       2610      22903
Examining 183 file(s)

                          Ohloh Line Count Summary

Language          Files       Code    Comment  Comment %      Blank      Total
----------------  -----  ---------  ---------  ---------  ---------  ---------
java                158       8846       2671      23.2%       1899      13416
xml                   1         14         18      56.2%          0         32
----------------  -----  ---------  ---------  ---------  ---------  ---------
Total               159       8860       2689      23.3%       1899      13448

......

@oldratlee
Copy link
Contributor

oldratlee commented Dec 4, 2018

comment coverage report of 1f751a6 generated by below script: @ralf0131
# excluded the licence comments at file beginning.

NO. module product code test code
1 dubbo-demo-api 0.0% (0/4) -
2 dubbo-serialization-fastjson 0.0% (0/182) -
3 dubbo-serialization-fst 0.0% (0/196) -
4 dubbo-serialization-protostuff 1.0% (3/311) -
5 dubbo-rpc-http 1.6% (3/186) 4.3% (9/202)
6 dubbo-registry-redis 1.7% (10/574) 0.0% (0/81)
7 dubbo-rpc-redis 1.9% (3/152) 1.7% (3/177)
8 dubbo-remoting-zookeeper 2.4% (12/489) 0.0% (0/269)
9 dubbo-rpc-webservice 2.4% (3/123) 12.4% (16/113)
10 dubbo-registry-zookeeper 2.9% (8/270) 3.3% (4/118)
11 dubbo-serialization-jdk 3.0% (12/385) -
12 dubbo-rpc-memcached 3.1% (3/93) 0.0% (0/3)
13 dubbo-rpc-hessian 3.3% (9/261) 3.8% (9/228)
14 dubbo-serialization-hessian2 3.6% (6/161) -
15 dubbo-registry-multicast 3.8% (15/384) 23.0% (44/147)
16 dubbo-remoting-mina 4.0% (23/555) 19.5% (34/140)
17 dubbo-container-log4j 4.1% (3/70) 20.0% (3/12)
18 dubbo-monitor-default 4.5% (17/357) 1.9% (6/317)
19 dubbo-remoting-netty 4.9% (52/1016) 10.9% (68/558)
20 dubbo-remoting-grizzly 5.0% (27/512) -
21 dubbo-rpc-dubbo 5.9% (173/2740) 5.3% (111/1996)
22 dubbo-rpc-rest 7.2% (58/753) 3.5% (11/303)
23 dubbo-serialization-kryo 7.3% (34/432) -
24 dubbo-rpc-thrift 7.4% (93/1162) 8.2% (677/7612)
25 dubbo-container-spring 7.7% (3/36) 17.6% (3/14)
26 dubbo-filter-validation 8.5% (33/356) 0.0% (0/163)
27 dubbo-rpc-injvm 9.9% (15/136) 5.9% (12/192)
28 dubbo-compatible 10.0% (245/2214) 0.7% (11/1575)
29 dubbo-rpc-rmi 10.4% (13/112) 33.3% (80/160)
30 dubbo-registry-default 11.3% (23/180) 10.1% (172/1538)
31 dubbo-remoting-p2p 11.8% (117/872) 0.0% (0/155)
32 dubbo-remoting-netty4 12.1% (168/1223) 7.4% (20/250)
33 dubbo-container-api 13.2% (12/79) -
34 dubbo-cluster 13.8% (408/2544) 7.5% (278/3421)
35 dubbo-registry-api 14.8% (350/2010) 13.9% (129/796)
36 dubbo-config-api 16.1% (762/3984) 4.8% (212/4167)
37 dubbo-filter-cache 16.4% (96/490) 0.0% (0/202)
38 dubbo-qos 17.0% (289/1412) 0.0% (0/728)
39 dubbo-common 18.5% (3414/15045) 1.8% (159/8846)
40 dubbo-monitor-api 18.8% (52/225) 2.8% (6/208)
41 dubbo-container-logback 19.2% (14/59) 13.0% (3/20)
42 dubbo-demo-provider 19.2% (5/21) -
43 dubbo-config-spring 19.4% (683/2843) 7.5% (238/2945)
44 dubbo-remoting-http 19.8% (79/319) 0.0% (0/60)
45 dubbo-remoting-api 20.1% (1545/6148) 6.0% (255/4016)
46 dubbo-test-spring3 20.2% (20/79) -
47 dubbo-demo-consumer 20.8% (5/19) -
48 dubbo-rpc-api 20.9% (1036/3924) 4.6% (76/1591)
49 dubbo-metrics-api 39.9% (405/609) 0.0% (0/129)
50 dubbo-serialization-api 69.5% (194/85) -

comment-coverage-report.sh:

https://gist.github.com/oldratlee/6d02a849b8b84a9aba26be7ee65f5560

@oldratlee
Copy link
Contributor

oldratlee commented Dec 4, 2018

@lovepoem
@oldratlee,can we statistics the comment coverage as test coverage in the codecov.io ?

I have not found comment coverage info in the codecov.io yet.

@khanimteyaz
Copy link
Contributor

Being a new to open source development mythology will be possible for some one to guide me how I can proceed further to help the community regarding the current issue.

@carryxyh
Copy link
Member Author

carryxyh commented Dec 5, 2018

@carryxyh
Copy link
Member Author

carryxyh commented Dec 5, 2018

@khanimteyaz
We are of course willing to help you with some work, very welcome!

some one to guide me how I can proceed further to help the community regarding the current issue

I recommend that you subscribe to our mailing list. U can subscribe the dev group and go there directly for help, send your questions, I will help you.
:)

@khanimteyaz
Copy link
Contributor

@carryxyh
Thanks for the greeting. I would do that.

@khanimteyaz
Copy link
Contributor

Created the pull request for addition of comments for dubbo-filter module

@khanimteyaz
Copy link
Contributor

khanimteyaz commented Dec 7, 2018

How can I create the card under project (https://github.com/apache/incubator-dubbo/projects/2) and assigned it to me, so that it can be easier for reported to track.

@ralf0131
Copy link
Contributor

@khanimteyaz Maybe you don't have the permission. Please leave a comment about what you'd like to add, we can help you do that!

Please create an issue for your work, so that I can add your issue the to the project.

@khanimteyaz
Copy link
Contributor

@ralf0131 I have created an issue #2935 (Creating dubbo dubbo-rpc-api java documentation) and will be on it. Claiming to work on this.

@ralf0131 ralf0131 moved this from To do to In progress in Enhance the comment of code. Dec 11, 2018
@khanimteyaz
Copy link
Contributor

@ralf0131 I have raised a pull request regarding the current thread issue #2921 ,should bring #2921 issue into this group and close it as well as it got merged to master?

@ralf0131
Copy link
Contributor

@khanimteyaz I've added #2921 to https://github.com/apache/incubator-dubbo/projects/2, and marked it as Done.

@khanimteyaz
Copy link
Contributor

@ralf0131 thanks for doing it.Appreciate it.

@khanimteyaz
Copy link
Contributor

khanimteyaz commented Dec 14, 2018 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Everything needs help from contributors
Projects
No open projects
Development

No branches or pull requests

7 participants