apache / incubator-hugegraph-computer

HugeGraph Computer - A distributed graph processing system for hugegraph (OLAP)
https://hugegraph.apache.org/docs/quickstart/hugegraph-computer/
Apache License 2.0
42 stars 41 forks source link

[Feature-241] Add PULL_REQUEST_TEMPLATE #242

Closed Radeity closed 1 year ago

Radeity commented 1 year ago

Purpose of the PR

codecov[bot] commented 1 year ago

Codecov Report

Merging #242 (14897e5) into master (cc5a7e7) will decrease coverage by 0.04%. The diff coverage is n/a.

@@             Coverage Diff              @@
##             master     #242      +/-   ##
============================================
- Coverage     85.83%   85.80%   -0.04%     
+ Complexity     3232     3231       -1     
============================================
  Files           344      344              
  Lines         12072    12072              
  Branches       1087     1087              
============================================
- Hits          10362    10358       -4     
- Misses         1185     1187       +2     
- Partials        525      527       +2     

see 3 files with indirect coverage changes

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

imbajin commented 1 year ago

simplify it first, gradually adjust it according to the usage/feedback

image

Also the PR title use the basic rule first (consider expand it in future)

@liuxiaocs7 could take a look

Radeity commented 1 year ago

Hi, @imbajin , IMHO, I don't recommend removing section Verifying these changes, how to test this pr is important, sometimes, it can not be tested by regular UT, also, it helps reviewer to know more about its degree of completion. Maybe we can simplify this section first.

imbajin commented 1 year ago

Hi, @imbajin , I don't recommend removing section Verifying these changes, how to test this pr is important, sometimes, it can not be tested by regular UT, also, it helps reviewer to know more about its degree of completion. Maybe we can simplify this section first, WDYT?

Fine, use a short 🩳 description for it first, thanks for the feedback

BTW, the reason why I simplify the template is because in the “markdown source code” mode, most new users will directly “select all + delete” the content😿 (same in title), we had a similar problem with our previous issue template, until I used a new UI refactoring and did not allow it to be deleted (but the New Table UI can't use in PR template)

Radeity commented 1 year ago

Fine, use a short 🩳 description for it first, thanks for the feedback

BTW, the reason why I simplify the template is because in the “markdown source code” mode, most new users will directly “select all + delete” the content😿 (same in title), we had a similar problem with our previous issue template, until I used a new UI refactoring and did not allow it to be deleted (but the New Table UI can't use in PR template)

Get your point :D, and i've modified some description, maybe this part can be improved, PTAL: https://github.com/Radeity/incubator-hugegraph-computer/blob/31ea3e02b731255f42e877dd82ee45d0a1f5182d/.github/PULL_REQUEST_TEMPLATE.md?plain=1#L51-L54

Radeity commented 1 year ago

nit: should we use close: #issue_number here to be consistent with L7 and L26

Hi, @liuxiaocs7 , thanks for reminding, I think use link here is better, so i've modified comment in L7 and L26 :D

liuxiaocs7 commented 1 year ago

LGTM, unify it in other repo later

Added in HG Tasks, good start for beginner by refering this pr!