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

Doc: create issue and PR management guidelines #10668

Merged
merged 1 commit into from
May 15, 2019

Conversation

spzala
Copy link
Member

@spzala spzala commented Apr 23, 2019

I would like to propose a formal guide for issue triage and PR management. This should help us keep open issues and PRs under a desirable number. For example, keep issues under 100. These guidelines should specially help manage and close issues and PRs that are inactive in a timely manner.
Sometimes back I had proposed triage guidelines to the Kubernetes community and created one working with the community. Here is a link for reference http://bit.ly/2W0PM4j In the community smaller projects and SIGs are also encouraged to create formal triage doc to keep issues and PRs numbers in check.

@spzala
Copy link
Member Author

spzala commented Apr 23, 2019

@philips @xiang90 @hexfusion @gyuho @jpbetz as I mentioned in the PR description, I have created this PR with two docs to start the discussion. I am hopping that these guidelines will evolve and should be helpful to contributors to understand the triage process, and mainly help us to close inactive issues and PRs in a timely manner to keep their number in check. I am open for suggestions and feedback. Thanks!

@codecov-io
Copy link

Codecov Report

Merging #10668 into master will increase coverage by 0.24%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #10668      +/-   ##
==========================================
+ Coverage   71.37%   71.61%   +0.24%     
==========================================
  Files         393      393              
  Lines       36629    36629              
==========================================
+ Hits        26145    26233      +88     
+ Misses       8633     8549      -84     
+ Partials     1851     1847       -4
Impacted Files Coverage Δ
pkg/fileutil/purge.go 65.9% <0%> (-6.82%) ⬇️
proxy/grpcproxy/watcher.go 85.71% <0%> (-4.09%) ⬇️
proxy/grpcproxy/watch.go 88.55% <0%> (-3.02%) ⬇️
proxy/httpproxy/director.go 80% <0%> (-2.86%) ⬇️
clientv3/concurrency/election.go 79.68% <0%> (-2.35%) ⬇️
raft/progress.go 94.23% <0%> (-1.93%) ⬇️
clientv3/op.go 72.5% <0%> (-1.25%) ⬇️
proxy/grpcproxy/lease.go 79.18% <0%> (-0.91%) ⬇️
raft/raft.go 91.54% <0%> (-0.66%) ⬇️
clientv3/watch.go 91.13% <0%> (-0.64%) ⬇️
... and 13 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 216808e...047fd23. Read the comment docs.

Copy link
Contributor

@hexfusion hexfusion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@spzala this looks good lets talk about this at next community meeting can you add this to the agenda please?

@spzala
Copy link
Member Author

spzala commented Apr 25, 2019

@spzala this looks good lets talk about this at next community meeting can you add this to the agenda please?

Absolutely! Thanks @hexfusion You read my mind :-) Just added it in the meeting agenda!


## Verify important labels are in place

Make sure that appropriate reviewers are added to the PR. Also make sure that milestone is identified. If any of these or other important labels are missing, add them. If correct label can not be decided, leave a comment for the maintainers of the SIG to do so if needed.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove the mentioning of SIG. no sig in etcd :P

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah..yeah..LOL. Thanks @xiang90 I will remove it!

@xiang90
Copy link
Contributor

xiang90 commented May 2, 2019

looks pretty good. we probably need to have bot to close PR/issue, ping assigned owner automatically.


## Validate if an issue is a bug

Validate if the issue is indeed a bug. If not, add a comment with findings and close trivial issue. For non-trivial issue, wait to hear back from issue reporter and see if there is any objection. If issue reporter does not reply in 30 days, close the issue.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some issues are reported with only error messages but with no reproduce steps, which makes it difficult to validate if it is indeed a bug. Maybe add something like "if more information is needed from issue reporter, ..."?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jingyih sure, that's a good idea, will do!

@spzala
Copy link
Member Author

spzala commented May 2, 2019

looks pretty good. we probably need to have bot to close PR/issue, ping assigned owner automatically.

@xiang90 thanks so much for the quick review and yes having bot for tasks you mentioned would be great. Also probably some labels down the road - for things like what @jingyih mentioned when more information is needed from issue reporter to reproduce or understand the issue better (e.g. labeling the issue with need-information) and possibly few other labels as we discussed in the meeting for easy identification and query. I will work on labels and reach out to @gyuho @hexfusion for bot. Thanks!

I would like to propose a formal guide for issue triage and PR management.
This should help us keep open issues and PRs under a desirable numbers.
For example, keep issues under 100. These guidelines should specially help
manage and close issues and PRs that are inactive in a timely manner.
@spzala
Copy link
Member Author

spzala commented May 15, 2019

@xiang90 @hexfusion @jingyih should we merge this if there are no more review comments, so that we have guidelines in place? Thanks!

@xiang90
Copy link
Contributor

xiang90 commented May 15, 2019

Sure. Let us merge this in first. We need to link two these two docs from main doc page or readme though.

@xiang90 xiang90 merged commit 42acdfc into etcd-io:master May 15, 2019
@spzala
Copy link
Member Author

spzala commented May 15, 2019

Sure. Let us merge this in first. We need to link two these two docs from main doc page or readme though.

@xiang90 thank you so much and yes, that's definitely a good idea!! I will work on it, let me try to find out where it fits and will send it for review. Thanks!

@philips
Copy link
Contributor

philips commented May 17, 2019

It would be great if someone could investigate using some of the Kubernetes Prow infrastructure to automate some of these things.

@spzala
Copy link
Member Author

spzala commented May 29, 2019

It would be great if someone could investigate using some of the Kubernetes Prow infrastructure to automate some of these things.

I will be investigating into Kubernetes Prow infrastructure for our need @philips @xiang90 Thanks!

spzala added a commit to spzala/etcd that referenced this pull request Jun 5, 2019
related: etcd-io#10668

Provide a ref to the issue and PR management doc from the README,
similar to other references we have provided in the README.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants