[#551] docs: update templates for flaky test and pull request #588

kaijchen · 2023-02-10T07:39:55Z

What changes were proposed in this pull request?

Update flaky test template and pr template.

Why are the changes needed?

Flaky test template
- Title is not consistent with other templates.
- Default parent issue is invalid.
PR template
- Make the instructions shorter to read.
- Let PR author remove the instructions so they will read (hopefully).
- Update instructions, and add example for PR title. (Resolves [Improvement][Style] add autolink to issues in PR title and commit msg #551)

Does this PR introduce any user-facing change?

No.

How was this patch tested?

No need.

codecov-commenter · 2023-02-10T07:58:21Z

Codecov Report

Merging #588 (9f419d1) into master (fdc2743) will increase coverage by 0.01%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master     #588      +/-   ##
============================================
+ Coverage     60.84%   60.85%   +0.01%     
- Complexity     1797     1798       +1     
============================================
  Files           214      214              
  Lines         12398    12398              
  Branches       1051     1051              
============================================
+ Hits           7543     7545       +2     
+ Misses         4445     4444       -1     
+ Partials        410      409       -1

Impacted Files	Coverage Δ
...pache/hadoop/mapreduce/task/reduce/RssFetcher.java	`92.30% <0.00%> (+1.53%)`	⬆️

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

jerqi

LGTM, thanks @kaijchen

.github/PULL_REQUEST_TEMPLATE

advancedxy · 2023-02-10T08:27:12Z

.github/PULL_REQUEST_TEMPLATE


+Please outline the changes and how this PR fixes the issue.


You should document these lines should be replaced.

Is it enough to put the tips in parentheses?

advancedxy · 2023-02-10T08:27:44Z

.github/PULL_REQUEST_TEMPLATE


+Fix: #


This is not clear. Please add more comments about this section.

This is for reminding PR author to link the issue.

I know you intention. But contributor may got confused about what this is.

How to change it? How about this?

Suggested change

Fix: #

Fix: #(issue id)

advancedxy · 2023-02-10T08:28:28Z

.github/PULL_REQUEST_TEMPLATE


+Please list the user-facing changes introduced by your change, including
+  1. change in user-facing APIs
+  2. addition or removal of property keys


if no user-facing changes introduced, simply write 'NO'

Added default answer: "No."

kaijchen · 2023-02-10T08:58:52Z

Thanks @advancedxy @jerqi for the review.
Although there might be some concerns, PPMC members and committers are able to edit the PR title and description.
I think we can give it a try.

advancedxy · 2023-02-13T07:41:45Z

.github/PULL_REQUEST_TEMPLATE

+     - "[MINOR] refactor: fix typo in variable name"
+     - "[MINOR] docs: fix typo in README"
+     - "[#255] test: fix flaky test NameOfTheTest"
+   Reference: https://gist.github.com/joshbuchea/6f47e86d2510bce28f8e7f42ae84c716


is it a good idea to reference a gist url here?

I prefer a formal URL, such as the conventional commit urk: https://www.conventionalcommits.org/en/v1.0.0/

Let's make it a bit clear:

We are proposing an PR title that prefixed with github's issut autolink then a conventional commits title?
The conventional commit could be referred: https://www.conventionalcommits.org/en/v1.0.0/

advancedxy · 2023-02-13T07:44:37Z

Thanks @advancedxy @jerqi for the review. Although there might be some concerns, PPMC members and committers are able to edit the PR title and description. I think we can give it a try.

Yeah. I think so. However is it possible for you to write a COMMITTER_GUIDE.md in the repo root dir? The COMMITTER_GUIDE should be how to modify the pr title/summary before merging a PR.

.github/PULL_REQUEST_TEMPLATE

kaijchen · 2023-02-16T12:58:58Z

Thanks @advancedxy and @jerqi for the review.

xianjingfeng · 2023-02-27T07:58:10Z

.github/ISSUE_TEMPLATE/flaky-test-report.yml

@@ -66,7 +66,7 @@ body:
  - type: textarea
    attributes:
      label: Parent issue
-      value: "#1733"
+      value: ""


value must be of type String and cannot be empty. @kaijchen

Thanks, can you submit a PR to fix it?
Maybe let's just remove this field?

…pache#588) ### What changes were proposed in this pull request? Update flaky test template and pr template. ### Why are the changes needed? 1. Flaky test template * Title is not consistent with other templates. * Default parent issue is invalid. 2. PR template * Make the instructions shorter to read. * Let PR author remove the instructions so they will read (hopefully). * Update instructions, and add example for PR title. (Resolves apache#551) ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? No need.

docs: update flaky test report template

074b9f1

kaijchen requested review from jerqi and advancedxy February 10, 2023 07:40

kaijchen changed the title ~~docs: update flaky test template and pr template~~ [MINOR] docs: update flaky test template and pr template Feb 10, 2023

kaijchen changed the title ~~[MINOR] docs: update flaky test template and pr template~~ [#551] docs: update flaky test template and pr template Feb 10, 2023

kaijchen changed the title ~~[#551] docs: update flaky test template and pr template~~ [#551] docs: update flaky test and pull request template Feb 10, 2023

kaijchen marked this pull request as draft February 10, 2023 08:02

docs: update PR template

3930ba4

kaijchen force-pushed the template branch from 8c4d45b to 3930ba4 Compare February 10, 2023 08:15

kaijchen marked this pull request as ready for review February 10, 2023 08:16

jerqi previously approved these changes Feb 10, 2023

View reviewed changes

advancedxy reviewed Feb 10, 2023

View reviewed changes

update PR template

8efff2b

kaijchen dismissed jerqi’s stale review via 8efff2b February 10, 2023 08:44

kaijchen added 3 commits February 10, 2023 16:52

fixup

0a482ea

fixup

6534989

fixup

95679cf

kaijchen changed the title ~~[#551] docs: update flaky test and pull request template~~ [#551] docs: update templates for flaky test and pull request Feb 10, 2023

update

9f419d1

advancedxy reviewed Feb 13, 2023

View reviewed changes

kaijchen commented Feb 13, 2023

View reviewed changes

.github/PULL_REQUEST_TEMPLATE Outdated Show resolved Hide resolved

Update .github/PULL_REQUEST_TEMPLATE

2ab41df

kaijchen commented Feb 16, 2023

View reviewed changes

.github/PULL_REQUEST_TEMPLATE Outdated Show resolved Hide resolved

Update .github/PULL_REQUEST_TEMPLATE

5955d99

advancedxy approved these changes Feb 16, 2023

View reviewed changes

kaijchen merged commit 4f07b0c into apache:master Feb 16, 2023

kaijchen deleted the template branch February 16, 2023 12:59

xianjingfeng reviewed Feb 27, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#551] docs: update templates for flaky test and pull request #588

[#551] docs: update templates for flaky test and pull request #588

kaijchen commented Feb 10, 2023 •

edited

Loading

codecov-commenter commented Feb 10, 2023 •

edited

Loading

jerqi left a comment

advancedxy Feb 10, 2023

kaijchen Feb 10, 2023

advancedxy Feb 10, 2023

kaijchen Feb 10, 2023 •

edited

Loading

advancedxy Feb 10, 2023

kaijchen Feb 10, 2023

advancedxy Feb 10, 2023

kaijchen Feb 10, 2023

kaijchen commented Feb 10, 2023

advancedxy Feb 13, 2023

kaijchen Feb 16, 2023

advancedxy Feb 16, 2023

advancedxy Feb 16, 2023

advancedxy commented Feb 13, 2023

kaijchen commented Feb 16, 2023

xianjingfeng Feb 27, 2023

kaijchen Feb 27, 2023

xianjingfeng Feb 27, 2023

[#551] docs: update templates for flaky test and pull request #588

[#551] docs: update templates for flaky test and pull request #588

Conversation

kaijchen commented Feb 10, 2023 • edited Loading

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

codecov-commenter commented Feb 10, 2023 • edited Loading

Codecov Report

jerqi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kaijchen Feb 10, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kaijchen commented Feb 10, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

advancedxy commented Feb 13, 2023

kaijchen commented Feb 16, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kaijchen commented Feb 10, 2023 •

edited

Loading

codecov-commenter commented Feb 10, 2023 •

edited

Loading

kaijchen Feb 10, 2023 •

edited

Loading