8000 Done markers · Issue #24 · kegsay/github-pull-review · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
/ github-pull-review Public
  • Notifications < 8000 tool-tip id="tooltip-8a050053-0f53-4bdf-9102-f840690185f2" for="repository-details-watch-button" popover="manual" data-direction="s" data-type="description" data-view-component="true" class="sr-only position-absolute">You must be signed in to change notification settings
  • Fork 3
  • Star 5

Done markers #24

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.

Sign up for GitHub

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

Jump to bottom
Open
8000
kegsay opened this issue Oct 16, 2015 · 2 comments
Open

Done markers #24

kegsay opened this issue Oct 16, 2015 · 2 comments
Labels
enhancement++

Comments

@kegsay
Copy link
Owner
kegsay commented Oct 16, 2015

Each actionable comment (that is, first comment on a line, or any overview comment with the text "Needs action:") has a check box "Done?". If you tick it, it replies with a comment "Done." (which is suppressed on the UI). Subsequent replies on line comments "undones" the comment thread. This is done by unticking the box (which prompts for a "Reason:").

Examples: (A=Reviewer, B=Reviewee)

Basic:

  • A: Post line comment [1 undone item]
  • B: Pushes commits
  • B: Ticks done box (replies with "Done.") [0 undone items]

Basic with non-actionable overview:

  • A: Posts line comments [2 undone items]
  • A: Posts "LGTM modulo comments" [2 undone items]
  • B: Pushes commits
  • B: Ticks "done" boxes [0 undone items]

Basic with actionable overview:

  • A: Posts line comments [2 undone items]
  • A: Posts actionable overview comment (Tick "Actionable?" box with prefixes comment with "Needs action:" [3 undone items]
  • B: Pushes commits
  • B: Ticks "done" boxes [0 undone items]

Ping-pong:

  • A: Posts line comment [1 undone item]
  • B: Ticks done box [0 undone items]
  • A: Posts on same line. This is undone as last comment isn't "Done." [1 undone item]
  • B:Ticks done box [0 undone items]
  • ...

Overview comments:

  • A: Post overview comment marked as "Actionable?".
  • B: Marks overview comment as Done (with optional reason) - This inserts the comment "[url to comment id] Done. [reason]" so we can pair up multiple actionable overview comments (line comments are easier since the line determines the thread).
@kegsay kegsay mentioned this issue Oct 17, 2015
Closed
@kegsay
Copy link
Owner Author
kegsay commented Oct 17, 2015

Structure:

class Action {
  Comments[] comments; // line comments or overview comments linked via comment ID URLs somewhere in the body
  boolean isDone; // true if last comment is "Done."
  string owner; // person who made the original comment
}

Processing the block of comments in a PR to actions:

  • Create new Action for each set of line comments on the same line.
  • Create new Action for each set of overview comments which start "Needs action:" and are linked via comment ID urls somewhere in the body.

@kegsay kegsay mentioned this issue Oct 17, 2015
Closed
@kegsay
Copy link
Owner Author
kegsay commented Nov 24, 2015

Good progress has been made on this but it's still not done (hurrhurr). Need tick box for "Done?" and "Actionable?" on overview comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement++
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@kegsay
0