Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(9)

Issue 1840833002: Revert of git cl: Rework Changelist class for Rietveld/Gerrit use. (Closed)

Created:
4 years, 8 months ago by dnj
Modified:
4 years, 8 months ago
CC:
chromium-reviews, dpranke+depot_tools_chromium.org, iannucci+depot_tools_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/tools/depot_tools.git@master
Target Ref:
refs/heads/master
Visibility:
Public.

Description

Revert of git cl: Rework Changelist class for Rietveld/Gerrit use. (patchset #3 id:40001 of https://codereview.chromium.org/1830973003/ ) Reason for revert: Speculative revert, see crbug.com/598428. Original issue's description: > git cl: Rework Changelist class for Rietveld/Gerrit use. > > This adds pluggable codereview-specific implementations into > Changelist class. The specific implementation is chosen at > Changelist automatically, with Rietveld being default for > backwards compatibility. > > Gerrit implementation for Gerrit is incomplete, and will be > added in later CLs. However, it is sufficient to ensure > current functionality of this tool is not diminished. > > Sadly, the base class isn't completely free from Rietveld > assumptions because of presubmit_support. Apparently, PRESUBMIT > scripts can make use of Rietveld instance for RPCs directly. > This use doesn't make sense for Gerrit, which substitutes > rietveld instance with a dummy object, which raises exception > on any attribute access with a diagnostic message. > > This also includes refactoring of some related code which > (ab)used ChangeList. Overall, this CL adds a few extra call to > git config in order to determine which codereview to use, but > but it shouldn't have any performance impact. > > > > These is a reland of these 4 CLs + a fix. > patch from issue 1827523003 at patchset 20001 (http://crrev.com/1827523003#ps20001) > patch from issue 1830703004 at patchset 1 (http://crrev.com/1830703004#ps1) > patch from issue 1830923002 at patchset 60001 (http://crrev.com/1830923002#ps60001) > patch from issue 1805193002 at patchset 380001 (http://crrev.com/1805193002#ps380001) > > > > R=machenbach@chromium.org,sergiyb@chromium.org,andybons@chromium.org > BUG=579160, 597638 > > Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=299506 TBR=andybons@chromium.org,machenbach@chromium.org,sergiyb@chromium.org,tandrii@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=579160, 597638 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=299515

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+157 lines, -428 lines) Patch
M git_cl.py View 21 chunks +131 lines, -397 lines 0 comments Download
M tests/git_cl_test.py View 12 chunks +26 lines, -31 lines 0 comments Download

Messages

Total messages: 6 (1 generated)
dnj
Created Revert of git cl: Rework Changelist class for Rietveld/Gerrit use.
4 years, 8 months ago (2016-03-28 22:25:03 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1840833002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1840833002/1
4 years, 8 months ago (2016-03-28 22:25:10 UTC) #2
commit-bot: I haz the power
Committed patchset #1 (id:1) as http://src.chromium.org/viewvc/chrome?view=rev&revision=299515
4 years, 8 months ago (2016-03-28 22:25:17 UTC) #4
tandrii(chromium)
thanks, Dan!
4 years, 8 months ago (2016-03-28 23:09:27 UTC) #5
tandrii(chromium)
4 years, 8 months ago (2016-03-29 09:24:48 UTC) #6
Message was sent while issue was closed.
A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/1844523002/ by tandrii@chromium.org.

The reason for reverting is: with a fix..

Powered by Google App Engine
This is Rietveld 408576698