git cl: Rework Changelist class for Rietveld/Gerrit use.

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

[tandrii(chromium), 2016-03-24 20:49:35]

Description was changed from

==========
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
==========

to

==========
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
==========

[Bons, 2016-03-24 20:50:11]

lgtm

https://codereview.chromium.org/1830973003/diff/20001/git_cl.py
File git_cl.py (right):

https://codereview.chromium.org/1830973003/diff/20001/git_cl.py#newcode1569
git_cl.py:1569: # This is a hack, to make presubmit_support think that rietveld
is not
no comma after hack

https://codereview.chromium.org/1830973003/diff/20001/git_cl.py#newcode1570
git_cl.py:1570: # defined, yet still ensure that calls directly result in decent
in a decent

[tandrii(chromium), 2016-03-24 20:50:26]

PTAL

PS1 is original CLs combined, because only first one of them was really big, the
other 3 were {refactoring, nit, 1line CloseIssue implementation).

PS2 has the actual fix.

[tandrii(chromium), 2016-03-24 20:51:59]

Thanks for review.

https://codereview.chromium.org/1830973003/diff/20001/git_cl.py
File git_cl.py (right):

https://codereview.chromium.org/1830973003/diff/20001/git_cl.py#newcode1569
git_cl.py:1569: # This is a hack, to make presubmit_support think that rietveld
is not
On 2016/03/24 20:50:11, Bons wrote:
> no comma after hack

Done.

https://codereview.chromium.org/1830973003/diff/20001/git_cl.py#newcode1570
git_cl.py:1570: # defined, yet still ensure that calls directly result in decent
On 2016/03/24 20:50:11, Bons wrote:
> in a decent

Done.

[Sergiy Byelozyorov, 2016-03-25 11:59:37]

lgtm

[tandrii(chromium), 2016-03-28 12:13:30]

The CQ bit was checked by tandrii@chromium.org

[tandrii(chromium), 2016-03-28 12:13:30]

The patchset sent to the CQ was uploaded after l-g-t-m from
andybons@chromium.org
Link to the patchset: https://codereview.chromium.org/1830973003/#ps40001
(title: "review")

[commit-bot: I haz the power, 2016-03-28 12:13:39]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1830973003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1830973003/40001

[commit-bot: I haz the power, 2016-03-28 12:15:55]

Description was changed from

==========
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
==========

to

==========
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
==========

[commit-bot: I haz the power, 2016-03-28 12:15:56]

Committed patchset #3 (id:40001) as
http://src.chromium.org/viewvc/chrome?view=rev&revision=299506

[dnj, 2016-03-28 22:25:03]

A revert of this CL (patchset #3 id:40001) has been created in
https://codereview.chromium.org/1840833002/ by dnj@chromium.org.

The reason for reverting is: Speculative revert, see crbug.com/598428..

[dnj, 2016-03-28 22:35:25]

On 2016/03/28 22:25:03, dnj wrote:
> A revert of this CL (patchset #3 id:40001) has been created in
> https://codereview.chromium.org/1840833002/ by mailto:dnj@chromium.org.
> 
> The reason for reverting is: Speculative revert, see crbug.com/598428..

This is confirmed to have caused the problem in crbug.com/598428

[tandrii(chromium), 2016-03-29 09:27:09]

https://codereview.chromium.org/1830973003/diff/40001/git_cl.py
File git_cl.py (right):

https://codereview.chromium.org/1830973003/diff/40001/git_cl.py#newcode911
git_cl.py:911: return self._load_codereview_impl(
this is missing **kwargs, that's why it broke the autorollers, which specify
auth_config options on command line.

https://codereview.chromium.org/1830973003/diff/40001/git_cl.py#newcode912
git_cl.py:912: codereview='gerrit' if settings.GetIsGerrit() else 'rietveld')
missing kwargs here, that's why this broke the autoroller, which specifies
refresh token on command line.

[tandrii(chromium), 2016-03-29 09:29:51]

Fixed reland is here: https://codereview.chromium.org/1838143002