Improve code contribution guidelines for patch authors
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	Aklapper
	Mar 7 2016, 11:50 AM

Description

This task item was mentioned as one topic in the Developer Summit session T114419 and hence is a subtask of T101686. See also T207.

Task summary to be improved - please edit.

Improve initial quality of contributed patchsets and communicate expectations.

Actions:

Clean up / rewrite https://www.mediawiki.org/wiki/Gerrit/Code_review/Getting_reviews (e.g. contains "Gerrit limitations" section which is irrelevant for a patch author); keep as generic / infrastructure-independent as possible (Gerrit vs Differential)
Incorporate author-related items from https://www.mediawiki.org/wiki/Gerrit/Code_review if appropriate
Check https://www.mediawiki.org/wiki/Gerrit/Commit_message_guidelines (and keep generic if possible - updating the page once Differential has replaced Gerrit is T123081: Update Commit Message Guidelines for Differential)
Potentially incorporate T121751: Document best practices to amend a change written by another contributor?

Intention: Improve the situation described in https://www.mediawiki.org/wiki/User:AKlapper_%28WMF%29/Code_Review#Poor_quality_of_contributors.27_patches (and to some extend https://www.mediawiki.org/wiki/User:AKlapper_%28WMF%29/Code_Review#Workload_of_existing_reviewers )

Related Objects
Search...

Status	Assigned	Task
Resolved	Qgil	T107562 Tech community KPIs for the WMF metrics meeting
Resolved	Dicortazar	T108816 Regression: "Ranking of repositories" in Gerrit Review Queue is broken
Duplicate	Qgil	T125545 Phabricator Q&A session for Community Liaisons
Resolved	Qgil	T116025 Goal: Align Community Liaison and Developer Relations project management practices
Resolved	Qgil	T119387 Community Liaison and Developer Relation quarterly goals for January - March 2016
Resolved	greg	T141066 Identify "first responders" for "all" "components" deployed on Wikimedia servers
Invalid	None	T98348 Implement the Wikimedia Foundation Call to Action 2015
Invalid	None	T98358 WMF to integrate, consolidate, and pause or stop stalled initiatives
Resolved	Qgil	T109829 Developer Relations quarterly goals for October-December 2015
Resolved	Aklapper	T78639 How to address the long tail of low priority tasks in active projects
Declined	None	T113707 Decision on Wikimedia Foundation service-level agreement on code review times
Declined	None	T113706 Define Cut-off date for abandoning old changesets in our code review tool
Open	None	T78768 Agree on and implement actions to prioritize code review of patches submitted by volunteers
Resolved	Aklapper	T101686 Goal: Define potential actions to reduce code review queues and waiting times
Resolved	Aklapper	T88531 Goal: Organize a Gerrit Cleanup Day on September 23, 2015
Resolved	Aklapper	T94578 Most basic Tech Community metrics are published and up to date
Resolved	Dicortazar	T86152 Active Gerrit users on a monthly basis
Resolved	Dicortazar	T59038 Basic metrics about contributors exercising +2/-2 permissions in Gerrit
Resolved	Acs	T92953 Migrate Korma identitites database to SortingHat
Resolved	Dicortazar	T96371 Community Metrics for IRC channels not updated since 09/2013
Resolved	Dicortazar	T97716 Gerrit changes reviewed per month (on scr.html)
Resolved	Dicortazar	T97717 Active changeset authors and changeset reviewers per month
Resolved	Dicortazar	T104176 404 for VizGrimoireJS/data/metrics.json on korma; makes certain pages unusable
Resolved	Dicortazar	T72600 "Age of unreviewed changesets by affiliation" shows negative number of changesets
Resolved	Dicortazar	T100189 "Median time to review for Gerrit Changesets, per month": External vs. WMF/WMDE/etc patch authors
Invalid	Dicortazar	T97715 Median time to review for Gerrit Changesets, per month
Resolved	Dicortazar	T111767 Automate the identities generation
Resolved	Dicortazar	T101777 Remove deprecated repositories from korma.wmflabs.org code review metrics
Resolved	Aklapper	T102920 What to do with old open patches for unmaintained/inactive repositories when not even the uploader responds
Resolved	Dicortazar	T108507 Patches with Verified -1 should not be counted as open in our code review metrics
Resolved	Dicortazar	T102112 Ranking of repositories in Korma's code review page should update more often
Duplicate	None	T113300 Enable `file:` queries in gerrit
Resolved	Dicortazar	T110947 Provide open changeset snapshot data on Sep 22 and Sep 24 (for Gerrit Cleanup Day)
Resolved	Bawolff	T114419 Event on "Make code review not suck"
Resolved	• mmodell	T128372 Document use of Owners in Phabricator and advertise it
Resolved	None	T140713 "Edit Paths" in Owners interface is unusable: Lists all repositories in an unsorted select box with no search
Resolved	• mmodell	T128371 Set up Code Review office hours
Resolved	Legoktm	T128370 Update ownership list on [[mw:Developers/Maintainers]]
Resolved	tstarling	T115852 Fix unclear maintenance responsibilities for some parts of MediaWiki core repository
Resolved	greg	T115854 Document code area responsibilities (Phab projects, Git repos) of each WMF engineering/dev team
Resolved	Aklapper	T129068 Improve code contribution guidelines for patch authors
Resolved	Aklapper	T134256 Make Wikimedia's Git/Gerrit documentation less horrible and less scattered
Resolved	Aklapper	T134126 Merge numeous "Git/Gerrit troubleshooting/problems" sections
Resolved	Aklapper	T129067 Document a structured, standardized approach for reviewing code contributions
Resolved	None	T129842 Launch the Wikimedia Code Review Working Group

Event Timeline

Aklapper created this task.Mar 7 2016, 11:50 AM

Aklapper added a parent task: T101686: Goal: Define potential actions to reduce code review queues and waiting times.

Aklapper updated the task description. (Show Details)

Aklapper updated the task description. (Show Details)Mar 7 2016, 11:55 AM

Aklapper mentioned this in T207: Update Code Review related documentation on wiki pages from Gerrit to Differential.

Aklapper mentioned this in T101686: Goal: Define potential actions to reduce code review queues and waiting times.Mar 7 2016, 1:17 PM

Ricordisamoa subscribed.Mar 7 2016, 1:49 PM

Aklapper updated the task description. (Show Details)Mar 7 2016, 7:14 PM

Qgil moved this task from To triage to Proposed: April-June 2016 on the Developer-Advocacy board.Mar 8 2016, 10:56 AM

Krenair subscribed.Mar 14 2016, 12:25 PM

I'm tentatively assigning this task to myself for the next quarter (April-June 2016) but if someone wants to work on this task earlier, please go ahead!

Aklapper mentioned this in T128448: Technical Collaboration quarterly goals for April - June 2016.Mar 14 2016, 8:51 PM

Qgil moved this task from Proposed: April-June 2016 to Apr-Jun-2016 on the Developer-Advocacy board.Apr 2 2016, 7:37 AM

Qgil edited projects, added Developer-Advocacy (Apr-Jun-2016); removed Developer-Advocacy.

Aklapper mentioned this in T78639: How to address the long tail of low priority tasks in active projects.Apr 3 2016, 11:54 AM

Aklapper mentioned this in T113706: Define Cut-off date for abandoning old changesets in our code review tool.Apr 3 2016, 12:04 PM

JanZerebecki subscribed.Apr 3 2016, 8:13 PM

Qgil moved this task from Backlog to Ready to Go on the Developer-Advocacy (Apr-Jun-2016) board.Apr 3 2016, 9:12 PM

Aklapper added a parent task: T78768: Agree on and implement actions to prioritize code review of patches submitted by volunteers.Apr 7 2016, 11:50 AM

Aklapper mentioned this in T78768: Agree on and implement actions to prioritize code review of patches submitted by volunteers.Apr 7 2016, 12:02 PM

Aklapper raised the priority of this task from Low to Medium.Apr 25 2016, 3:18 PM

Aklapper mentioned this in T134126: Merge numeous "Git/Gerrit troubleshooting/problems" sections.May 1 2016, 6:53 PM

Qgil edited projects, added Developer-Advocacy; removed Developer-Advocacy (Apr-Jun-2016).May 3 2016, 10:15 AM

Aklapper mentioned this in T134256: Make Wikimedia's Git/Gerrit documentation less horrible and less scattered.May 3 2016, 1:30 PM

Aklapper added a subtask: T134256: Make Wikimedia's Git/Gerrit documentation less horrible and less scattered.

Qgil edited projects, added Developer-Advocacy (Apr-Jun-2016); removed Developer-Advocacy.May 4 2016, 11:56 AM

Aklapper mentioned this in T73357: Add a welcome bot to Gerrit for first time contributors.May 13 2016, 5:54 PM

greg awarded a token.May 13 2016, 6:06 PM

Note to myself: Make sure https://www.mediawiki.org/wiki/Development_policy#Database_patches and https://www.mediawiki.org/wiki/Development_policy#Documentation_policy are linked.

Note to myself: Make sure that https://www.mediawiki.org/wiki/Gerrit/Tutorial#How_code_is_reviewed_in_Gerrit only covers technical, Gerrit specific, review aspects and is different from "generic" https://www.mediawiki.org/wiki/Gerrit/Code_review/Getting_reviews

Aklapper closed subtask T134256: Make Wikimedia's Git/Gerrit documentation less horrible and less scattered as Resolved.May 23 2016, 11:21 AM

Aklapper raised the priority of this task from Medium to High.May 30 2016, 8:16 AM

As a start, I've shortened the existing text and added a "Prerequisites" section: https://www.mediawiki.org/w/index.php?title=Gerrit%2FCode_review%2FGetting_reviews&type=revision&diff=2125544&oldid=1910448

Qgil moved this task from Ready to Go to May on the Developer-Advocacy (Apr-Jun-2016) board.May 31 2016, 7:36 AM

Aklapper moved this task from May to June on the Developer-Advocacy (Apr-Jun-2016) board.Jun 1 2016, 11:06 AM

Planning to resolve this task this week.
Watch https://www.mediawiki.org/wiki/Gerrit/Code_review/Getting_reviews for changes.

Things take longer (sigh) but https://www.mediawiki.org/wiki/Gerrit/Code_review/Getting_reviews now lists prerequisite expectations; expectations about the patch (scope, testing, docs); and dealing with obstacles (no timely review and further reasons for rework or rejection).

28 edits later:
https://www.mediawiki.org/w/index.php?title=Gerrit%2FCode_review%2FGetting_reviews&type=revision&diff=2163333&oldid=1910448

I'm closing this task as it seems sufficient.

Further improvements encouraged. Dropped an email on wikitech-l about this.

Aklapper mentioned this in T129067: Document a structured, standardized approach for reviewing code contributions.Jun 26 2016, 4:58 PM

Qgil awarded a token.Jun 28 2016, 8:34 PM

Aklapper mentioned this in T149639: Better code review processes for reviewers.Dec 7 2016, 6:58 PM