cEP-0019.md: Meta-review system #123

li-boxuan · 2018-05-06T04:21:59Z

Describes how meta-reviews could be tracked and handled.

Closes #121

jayvdb · 2018-05-06T16:19:21Z

cEP-0019.md

+Background
+----------
+
+People including the author of pull request respond to comments by attaching


Keep this first sentence (and probably first paragraph) factually describing the feature we are going to use. It is called reactions in GitHub.
Dont mentioned meta-review in the first sentence, which is a way of using reactions.

It is also worth describing what are the types of comments which might appear on a PR, and what is meant by "review comments".

Note there is a technical limitation in that reactions can not be placed onto the "review summary", which means we may need to describe what types of feedback should be put into the "review summary" text box, as it will be not able to be meta-reviewed.

These first few sentences need to be revised slightly so that they factual describe reactions.

dont mention emoji in the first sentence.

"author of pull request" is not especially relevant to reactions, as they can appear on issues, etc., so it should not be part of the first sentence.

How do you explain "reactions" for someone who hasnt seen them before?

Are reactions supported by other VCS hosters?

jayvdb · 2018-05-06T16:20:12Z

cEP-0019.md

+----------
+
+People including the author of pull request respond to comments by attaching
+emojis. Those emojis are called meta-review, or


Those emojis are called meta-review

This isnt correct.

We are going to use reactions as a mechanism to build a meta-review system.

jayvdb · 2018-05-06T16:26:03Z

cEP-0019.md

+do meta-review.
+
+Proposed Approach
+-----------


add more -

jayvdb · 2018-05-06T16:33:44Z

cEP-0019.md

@@ -0,0 +1,208 @@
+Meta-review System
+===========================


too long.
see #125

jayvdb · 2018-05-06T16:35:00Z

cEP-0019.md

+### Collection of Meta-reviews
+
+Meta-review info, namely reactions, can be fetched via GitHub API V4 GraphQL.
+A Sample query is as follows:


Sample -> sample

Also include the returned result data for this sample query, as that will help the reader understand it.

jayvdb · 2018-05-06T16:36:15Z

cEP-0019.md

+
+### Analysis of Meta-reviews
+
+This paragraph explains how meta-review information can be utilized to


... paragraph ...

That doesnt make sense here, as this paragraph has only one sentence and it doesnt explain anything.

Should be section instead of paragraph. 😅

jayvdb · 2018-05-06T16:37:45Z

cEP-0019.md

+4. Display visitor's received meta-reviews if the visitor logs in.
+GitHub won't send a notification to a user if he receives any meta-review.
+Suppose I ask for someone's agreement and he uses 👍 to show his agreement.
+But I wouldn't receive any notification. I have to check that page regularly.


The lack of notifications could be mentioned in the background.

A real feature is: gh-board provides notifications of meta-reviews.

This could be done using a per-user atom feed.

Is anyone else providing solutions for this?
Maybe we can implement another tool to solve this?

I didn't find any third-party tool for that. The interesting point is that it seems the notification for reactions was open by default before. isaacs/github#631

jayvdb · 2018-05-06T16:52:19Z

gh-board supports columns, we are not using at the moment as they require different labels. We could create a column for 'meta-review required'

jayvdb · 2018-05-06T16:59:05Z

An objective of this project is that we can add more criteria to the developer processes, e.g. the reviews conducted by the newcomer must have been meta-reviewed and met some threshold of usefulness before they can become a developer.

A similar objective can be applied to start a criteria for becoming a maintainer.

These are probably separate cEPs, and the cEP for newcomer->developer needs to be written primarily by @sks444 , and you would request some small additions for your project.

Also you might like to consider doing some of the analysis of meta-review in the coala/community repo, so that the gh-board project is kept to be more about the interactive & user-friendly UI.

li-boxuan · 2018-05-07T03:52:31Z

Also you might like to consider doing some of the analysis of meta-review in the coala/community repo, so that the gh-board project is kept to be more about the interactive & user-friendly UI.

That sounds good but is really difficult to implement. For example, one necessary step to generate analysis is to count the score a person gets according to meta-review system metrics. If it is done in the coala/community repo, then it has to fetch the raw data from coala/gh-board repo. When gh-board wants to display visualization, it has to again fetch the analyzed data from coala/community repo. A problem is: how can we get up-to-date information? It sounds like the whole procedure has to be synchronous.

jayvdb · 2018-05-07T07:22:16Z

then it has to fetch the raw data from coala/gh-board repo.

It already does fetch issues.json. Really easy to fetch more data and do more analysis.

When gh-board wants to display visualization, it has to again fetch the analyzed data from coala/community repo.

This is what I am suggesting doesnt happen. The visualisation of statistics could be done in the community repo. Visualisation of statistics isnt very well suited for gh-board, as it is a UI for finding & interacting with issues. Also gh-board is a generic tool, while any analysis would depend on per-community settings.

li-boxuan · 2018-05-07T07:29:15Z

But what about in-app fetch? When a new/updated pull request is fetched by gh-board, shouldn't we update the "ranking list" column and "comments need review" column? If the analysis part is done in coala/community repo, how could we update them?

jayvdb · 2018-05-07T23:41:36Z

cEP-0019.md

+Background
+----------
+
+People including the author of pull request respond to comments by attaching


These first few sentences need to be revised slightly so that they factual describe reactions.

dont mention emoji in the first sentence.

"author of pull request" is not especially relevant to reactions, as they can appear on issues, etc., so it should not be part of the first sentence.

How do you explain "reactions" for someone who hasnt seen them before?

Are reactions supported by other VCS hosters?

jayvdb · 2018-05-07T23:44:12Z

cEP-0019.md

+meta-review system, similar to a meta-moderation system. These responses
+are to be collected, processed and displayed on
+[gh-board](https://github.com/coala/gh-board), which is a nice serverless
+kanban board.


kanban board for GitHub.

Also 'nice'? - why is it nice? Have you looked for other ones?

Why are we using a GitHub only tool for this project? When coala has lots of GitLab projects?

Better add somewhere that GitLab is beyond scope of this project, and why... ? ;-)

I like this style of review comments. 😄

jayvdb · 2018-05-07T23:46:40Z

cEP-0019.md

+
+Also, to encourage people to do more meta-reviews, statistics of meta-reviews
+are to be collected and analysed. People who do meta-reviews will get scores
+according to some metrics. A ranking list is to be displayed on gh-board.


A ranking list will be provided on the community site.

jayvdb · 2018-05-07T23:48:50Z

cEP-0019.md

+according to some metrics. A ranking list is to be displayed on gh-board.
+
+To conclude, this project builds a meta-review system on gh-board to display
+statistics (score ranking, meta-reviews in need), and encourage people to


"meta-reviews in need" is one of the primary objectives, and should be a separate paragraph.
This is the part which is UI, and must be in gh-board.

jayvdb · 2018-05-07T23:55:58Z

cEP-0019.md

+
+#### Ranking list
+
+A ranking list would be displayed on a separate column on gh-board, according


remove "on a separate column on gh-board"

gh-board is a kanban board. columns in kanban board have a very important function, and none of them are 'showing statistics'.

Where the ranking list is located is still open for discussion, but it can not be a column of the kanban board.

The part of the project which may be suitable for a kanban column is "PRs needing meta-review", which is below.

Agree. Putting it on community website is a much better idea.

jayvdb · 2018-05-08T00:03:50Z

cEP-0019.md

+
+#### Meta-reviews in Need
+
+Review comments that need meta-review will be displayed on a separate column


When is a PR needing meta-review? after the review comment, or after the PR is merged/closed?

The accuracy of a review comment is not known until the PR is merged or closed.

However the usefulness of a review comment to the PR author might be high , even if the review comment was not 100% correct.

So there are two, possibly three, perspectives.

meta-review by the author.

meta-review by other people in "developers" group, excluding author, (assuming developers group includes all maintainers also) where the meta-review is done before merge/close

meta-review by maintainers only, after merge/close. These are the real gold. They will reflect the accuracy of the review comments.

There is also the potential for reverted commits. A reverted commit should trigger a new meta-review of the original commit, as time may have proved that the merge was incorrect (or maybe the merge was a self-aware time-sensitive workaround, and is no longer needed).

jayvdb · 2018-05-08T00:07:42Z

cEP-0019.md

+4. Display visitor's received meta-reviews if the visitor logs in.
+GitHub won't send a notification to a user if he receives any meta-review.
+Suppose I ask for someone's agreement and he uses 👍 to show his agreement.
+But I wouldn't receive any notification. I have to check that page regularly.


A real feature is: gh-board provides notifications of meta-reviews.

This could be done using a per-user atom feed.

Is anyone else providing solutions for this?
Maybe we can implement another tool to solve this?

jayvdb · 2018-05-08T00:12:28Z

"comments need review" does not rely on statistics/ranking/scoring. It needs github data. Per comments above, it may also need some way to differentiate between developer/maintainer. But people with write access on repo is good enough for 'developer', and we'll need to be careful about what we do with 'maintainer'.

"my issues/PRs" and sub-views of that doesnt appear to be a feature of gh-board yet, but it should be. Create issue?

"ranking list" is the feature which does not need to be part of gh-board.

li-boxuan · 2018-05-09T05:17:18Z

It's easy to judge if one is a developer or not: only people in the organization can query the following information, otherwise nothing would be returned.

{
  organization(login: "coala") {
    teams(first: 100) {
      totalCount
    }
  }
}

But it's hard to differentiate maintainers from developers. Since the number of maintainers is usually small, maybe we can write that down in a config file? Also we could do some pre-fetching, e.g. query the teams and see if there is some team with name like 'maintainer', 'admin'.

gitmate-bot · 2018-05-12T04:26:08Z

cEP-0019.md

+
+One goal of this project is to encourage people to do meta-reviews,
+systematically. Meta-reviews in need will be displayed on gh-board.
+http://coala.io/metareview would be redirected to gh-board page which


The text does not comply to the set style.

Origin: MarkdownBear, Section: markdown.

The issue can be fixed by applying the following patch:

--- a/tmp/tmplg2p3lg9/cEP-0019.md +++ b/tmp/tmplg2p3lg9/cEP-0019.md @@ -79,7 +79,7 @@ One goal of this project is to encourage people to do meta-reviews, systematically. Meta-reviews in need will be displayed on gh-board. -http://coala.io/metareview would be redirected to gh-board page which +<http://coala.io/metareview> would be redirected to gh-board page which shows meta-reviews in need. The implementation details are discussed in the next section.

The commit d2a9108 has fixed this issue, but gitmate is still reporting the previous commit.

ya, possibly a bug in gitmate.

jayvdb · 2018-05-13T07:32:28Z

cEP-0019.md

+
+However, those reactions are used in an adhoc way. They are spread over all PRs
+and are not collected and analyzed. Moreover, GitHub doesn't send
+notifications to reviewers who receive reactions. By tracking reactions, a nice


Worth noting isaacs/github#631 , where you've asked for existence of tools

jayvdb · 2018-05-13T07:37:35Z

cEP-0019.md

+3. Review Discussion
+   [see sample](https://github.com/coala/gh-board/pull/18#discussion_r186249423)
+
+However, those reactions are used in an adhoc way. They are spread over all PRs


Here is probably a good spot to note that they were added to GitHub in March 2016.
https://blog.github.com/2016-03-10-add-reactions-to-pull-requests-issues-and-comments/

So we have a significant number of them on our issues -- enough to analyse existing behaviour and determine that they are useful for meta-review, but not enough analysis done to know the correct balance for recommended usages.

jayvdb · 2018-05-13T07:47:01Z

cEP-0019.md

+
+Then final score S would be:
+
+S = k * (P1 - N1) + c1 * P2 + c2 \* N2


Instead of 'The formula' in the first paragraph, you can indicate that those are some of the metrics which will be collected, and be available for the ranking formula.

Then here the formula can be explained as an example of how those metrics might be combined to create the final ranking.

Might be worth re-iterating that the ranking formula will reside in the community project, and can be modified over time in order to achieve different results.

jayvdb · 2018-05-13T07:57:40Z

cEP-0019.md

+Also, to encourage people to do more meta-reviews, statistics of meta-reviews
+are to be collected and analyzed. People who do meta-reviews will get scores
+according to some metrics. A ranking list is to be displayed on the community
+site. The newcomer guide will also be updated to require newcomers to finish


The newcomer guide ...

New paragraph for this.

And indicate that this project will provide the system so that the newcomer process is able to be changed, after testing it with newcomers and developers and maintainers, to required at least one meta-review and receive at least one positive meta-review. This is a minimal roll-out of the new system.

The newcomer guide doesnt need to be mentioned, as that is just one place where the newcomer process is documented.

The important aspect here is that change to the process will require a maintainer group endorsement of the functionality, so this project must have built a system that is ready for production, even if only a limited minimum implementation. After project close, every single coala person will be using this system (except the drive-by contributor who has no interest in our newcomer process).

jayvdb · 2018-05-13T08:21:32Z

cEP-0019.md

+
+When does a review need meta-review? There are two perspectives:
+
+1. Meta-review by developers, i.e. all people within the organization. The


Meta-review by developers, i.e. all people within the organization

That is confusing wrt coala terminology of newcomers.

Especially considering that this cEP proposes that newcomers will be required to do one meta-review and received a positive meta-review.

While you try to clarify it in the next sentence, it shows there is a fundamental problem with using these terms ;-)

Instead, these are meta-review during development of the PR, and meta-review after the merge. In the future we might also include meta-review after the release.

Then note that meta-review after the merge will be limited to maintainers, and meta-reviews by others will not be counted. This means that development meta-reviews are supposed to be done while development is happening, and not after the fact. This helps the 'context' problem.

Also we need to factor in deleted comments here. Need to include some background on who GitHub lets delete comments and when - I'm not familiar with those rules as I am always able to delete most comments, but some I can not. However I know that when I am on other repos, I do get to delete comments, but I've never played with that.

We should allow people to delete their own review comments if it is done soon after the comment, but it is implicitly a bad comment if the author deletes it. We could have automated meta-reviewing capture that.

Also editing a review comment after it has been seen by others (e.g. by email) is relevant for this project.

Where deleting/editing is definitely within scope of this project is after the review comment has been meta-reviewed. Storing information in the community repo means that we can have history which is lost on Github. We dont want to discard meta-review data simply because it was deleted from GitHub. And if a comment has been meta-reviewed, and the author can delete their comment, they are destroying feedback from the community when they delete their comment. That needs to warned against, and prevented, and possibly have a negative score given to it.

Also worth mentioning, but perhaps declaring out of scope, is self-meta-reviewing. While the gh-board queue wouldnt promote this, it is easy to do in the GitHub UI, and can be quite effective to do that instead of a person deleting/editing their own comment.

We should allow people to delete their own review comments if it is done soon after the comment, but it is implicitly a bad comment if the author deletes it. We could have automated meta-reviewing capture that.

This doesn't sound realistic to me. We fetch reactions probably via a daily cron job, which means we are not able to monitor pull requests.

jayvdb · 2018-05-13T08:27:06Z

cEP-0019.md

+   time. In particular, one will always see the review comment that is under
+   his PR first.
+
+2. Meta-review by maintainers. For maintainers, review comments after


Yes, the commit disappears on GitHub, completely, if it hasnt been mentioned. fwiw, GitLab keeps them.
If a bot mentions the commit sha, GitHub keeps the commit even if it is no longer in the PR.
So we may need a bot which mentions the sha when the commits are pushed, or when first reviewed, so that latter force pushes dont delete the content.

mentioning every commit when pushed is not ideal, as it is common for people to notice an error quickly and fix it themselves before any reviews. Keeping a history of them all may encourage people to be more careful before pushing, but likely will make them a bit more scared of pushing, and reluctant to do it.

jayvdb · 2018-05-13T08:31:46Z

cEP-0019.md

+
+2. Offer an option for users to do meta-reviews directly on gh-board. This
+   might be a bad idea - people shouldn't do meta-review unless they know the
+   whole PR context. This might be a good idea - in the future users can do


Oh, that is a good point. We dont want gitmate comments to be meta-reviewed, so that is an exception that needs to be built into the system ;-)

But what I mean is that gh-board can have a lighter UI than github for doing meta-reviews. Typically someone doing a meta-review will not need the 'whole PR context', but only needs a subset. Which subset they need is going to be hard to guess, but it doesnt matter if they gh-board guess are not always right. Worst case, the meta-reviewer loads the PR in GitHub and does it there. gh-board can use 80/20 rule - make the easy ones very easy, and the hard ones it can safely ignore.

li-boxuan · 2018-05-13T14:04:54Z

Please also take a look at mockup frontend here: https://gitlab.com/coala/GSoC/GSoC-2018/issues/61

jayvdb · 2018-05-13T17:47:10Z

cEP-0019.md

+   the merge may be without the necessary context to be properly evaluated.
+   However, if the commit SHA is mentioned, GitHub will keep the commit even
+   if it is no longer in the PR. A gitmate plugin is needed in the future to
+   make meta-reviews by maintainers more effective.


A gitmate plugin is needed ... to ... mention any SHAs which were reviewed, so that meta-reviews by maintainers will have more context and be more effective.

jayvdb

Needs approval by mentors.

Describes how meta-reviews could be tracked and handled. Closes coala#121

hemangsk · 2018-05-14T04:29:14Z

ack e9f7587

jayvdb · 2018-05-14T05:05:59Z

@gitmate-bot ff

gitmate-bot · 2018-05-14T05:06:01Z

Hey! I'm GitMate.io! This pull request is being fastforwarded automatically. Please DO NOT push while fastforward is in progress or your changes would be lost permanently ⚠️

gitmate-bot · 2018-05-14T05:06:11Z

Automated fastforward failed! Please fastforward your pull request manually via the command line.

Reason:

Command: git push upstream HEAD:master

Exit Code: 1

Cause:

remote: error: GH006: Protected branch update failed for refs/heads/master.        
remote: error: Required status check "review/gitmate/manual" is pending.        
To <hidden_oauth_token>
 ! [remote rejected] HEAD -> master (protected branch hook declined)
error: failed to push some refs to '<hidden_oauth_token>'

jayvdb · 2018-05-14T05:06:29Z

ack e9f7587

jayvdb · 2018-05-14T05:07:51Z

gitmate playing up again; made that check optional.

jayvdb · 2018-05-14T05:08:01Z

@gitmate-bot ff

gitmate-bot · 2018-05-14T05:08:03Z

Hey! I'm GitMate.io! This pull request is being fastforwarded automatically. Please DO NOT push while fastforward is in progress or your changes would be lost permanently ⚠️

gitmate-bot · 2018-05-14T05:08:14Z

Automated fastforward with GitMate.io was successful! 🎉

gitmate-bot added process/pending review size/S difficulty/newcomer labels May 6, 2018

jayvdb requested changes May 6, 2018

View reviewed changes

jayvdb added process/wip and removed process/pending review labels May 6, 2018

jayvdb added enhancement and removed difficulty/newcomer labels May 6, 2018

li-boxuan force-pushed the meta-review-cEP branch from 3f9df86 to a266680 Compare May 7, 2018 03:38

gitmate-bot added process/pending review size/M difficulty/newcomer and removed process/wip size/S labels May 7, 2018

gitmate-bot added New cEP and removed difficulty/newcomer labels May 7, 2018

jayvdb requested changes May 8, 2018

View reviewed changes

jayvdb added process/wip and removed process/pending review labels May 8, 2018

li-boxuan mentioned this pull request May 9, 2018

Display my issues/PRs on gh-board coala/gh-board#35

Open

li-boxuan force-pushed the meta-review-cEP branch from a266680 to 6558a76 Compare May 9, 2018 06:36

gitmate-bot added process/pending review and removed process/wip labels May 9, 2018

li-boxuan force-pushed the meta-review-cEP branch from eb43935 to d2a9108 Compare May 12, 2018 04:26

gitmate-bot reviewed May 12, 2018

View reviewed changes

sks444 mentioned this pull request May 12, 2018

cEP-0020: Newcomer metrics and Gamification system #131

Closed

jayvdb requested changes May 13, 2018

View reviewed changes

jayvdb added process/wip and removed process/pending review labels May 13, 2018

li-boxuan force-pushed the meta-review-cEP branch from d2a9108 to 57525c6 Compare May 13, 2018 13:59

gitmate-bot added process/pending review and removed process/wip labels May 13, 2018

jayvdb reviewed May 13, 2018

View reviewed changes

jayvdb previously approved these changes May 13, 2018

View reviewed changes

blazeu approved these changes May 14, 2018

View reviewed changes

li-boxuan dismissed jayvdb’s stale review via 3b068a5 May 14, 2018 03:50

li-boxuan force-pushed the meta-review-cEP branch from 57525c6 to 3b068a5 Compare May 14, 2018 03:50

cEP-0019.md: Meta-review system

e9f7587

Describes how meta-reviews could be tracked and handled. Closes coala#121

li-boxuan force-pushed the meta-review-cEP branch from 3b068a5 to e9f7587 Compare May 14, 2018 04:05

jayvdb approved these changes May 14, 2018

View reviewed changes

gitmate-bot merged commit e9f7587 into coala:master May 14, 2018

li-boxuan deleted the meta-review-cEP branch May 14, 2018 05:29

li-boxuan mentioned this pull request May 16, 2018

cEP-0026.md: Adds optimize caching cEP #146

Merged

		@@ -0,0 +1,208 @@
		Meta-review System
		===========================


		### Analysis of Meta-reviews

		This paragraph explains how meta-review information can be utilized to


		#### Ranking list

		A ranking list would be displayed on a separate column on gh-board, according


		#### Meta-reviews in Need

		Review comments that need meta-review will be displayed on a separate column


		Then final score S would be:

		S = k * (P1 - N1) + c1 * P2 + c2 \* N2


		When does a review need meta-review? There are two perspectives:

		1. Meta-review by developers, i.e. all people within the organization. The

cEP-0019.md: Meta-review system #123

cEP-0019.md: Meta-review system #123

Conversation

li-boxuan commented May 6, 2018

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

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

Choose a reason for hiding this comment

jayvdb commented May 6, 2018 • edited Loading

jayvdb commented May 6, 2018

li-boxuan commented May 7, 2018

jayvdb commented May 7, 2018

li-boxuan commented May 7, 2018

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

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

jayvdb commented May 8, 2018 • edited Loading

li-boxuan commented May 9, 2018 • 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

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

Choose a reason for hiding this comment

li-boxuan commented May 13, 2018

Choose a reason for hiding this comment

jayvdb left a comment

Choose a reason for hiding this comment

hemangsk commented May 14, 2018

jayvdb commented May 14, 2018

gitmate-bot commented May 14, 2018

gitmate-bot commented May 14, 2018

jayvdb commented May 14, 2018

jayvdb commented May 14, 2018

jayvdb commented May 14, 2018

gitmate-bot commented May 14, 2018

gitmate-bot commented May 14, 2018

jayvdb commented May 6, 2018 •

edited

Loading

jayvdb commented May 8, 2018 •

edited

Loading

li-boxuan commented May 9, 2018 •

edited

Loading