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

Establish best practices for code/PRs review #16

Open
yarikoptic opened this issue May 27, 2021 · 1 comment
Open

Establish best practices for code/PRs review #16

yarikoptic opened this issue May 27, 2021 · 1 comment
Labels
sec-contributing Section on contributing to DANDI

Comments

@yarikoptic
Copy link
Member

E.g.

related: #15 discussion

@yarikoptic
Copy link
Member Author

good old issue brought up some relevant discussion recently. Before it disappears in the annals of slack, here some ideas:

  • PR1: per discussion elsewhere I have agreed that **PRs in the dandi-archive should be merged by corresponding authors **. FYI: in dandi-cli it is primarily me or the author or could be anyone else on the team if it was approved but not merged.
  • PR2: I would like to discuss now how comments on PRs should be handled. IMHO comment threads in the PRs should be marked resolved by the corresponding author of the comment, and not the author of the PR, unless they got stale and comment author did not follow up. The rationale: I find it difficult to find among (possible multitude of) resolved comments the one I want to follow up (and possibly even Unresolve) on because I (as comment author) do not consider it completely addressed. WDYT?
  • PR3: IMHO commits (and ideally entire PRs) should not include unrelated to that commit/PR changes (beyond possibly obvious typo fixes if few etc) . It prohibits proper annotation of changes (as in commit/PR messages/descriptions), making it more difficult to review especially in restrospect (via git log and git blame)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sec-contributing Section on contributing to DANDI
Projects
None yet
Development

No branches or pull requests

1 participant