New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 600469 link

Starred by 10 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug


Sign in to add a comment

Turn Rietveld read-only

Project Member Reported by aga...@chromium.org, Apr 4 2016

Issue description

The infra team has an eventual goal of migrating everything from Rietveld to Gerrit. This is a tracking bug for the end of that effort: when all of our code reviews have moved to gerrit and Rietveld will be left as an archive of previous review activity.
 
Showing comments 26 - 125 of 125 Older
Blocking: 541293
Blocking: 457971
Blocking: 464471
Blocking: 465402
Blocking: 395626
Blocking: 525042
Blocking: 484420
Blocking: 627339
Blocking: 627574
Blocking: 612417
Blocking: -612417
Blocking: 627678
Blocking: 627738
Valid concerns raised in the initial announcement email:

It's useful to be able to comment on previously-landed CLs, e.g. "why was this line written like this" or "this breaks Foo", and have the comments sent out to the original author and reviewers. Will that be impossible in read-only mode?

Will the ability to revert from Rietveld be maintained (even if it creates a CL in Gerrit)? This is probably only important during the transition period but it's a useful tool for sheriffs/gardeners.

(Both of these would be non-issues if a migration of changes to Gerrit happens before too long after the switch.)
Cc: jparent@chromium.org
 Issue gerrit:3652  has been merged into this issue.
Blocking: 432905
Blocking: -484420
Blocking: 467152
Blocking: 492216
Blocking: 488713
Blocking: 426253
Blocking: 633631
Blocking: 634946
Blocking: 510944
Blocking: 627621
Blocking: 624545
Blocking: 614133
Blocking: 612257
Blocking: 610856
Blocking: 609590
Blocking: 605984
Blocking: 605712
Blocking: 603754
Blocking: 603311
Blocking: 596111
Blockedon: 583679
Blocking: 500032
Blocking: 608637
Blocking: 639817
Blocking: -627621
Blocking: 641987
Blocking: 642006
Blocking: 634942
Blocking: 637561
Blocking: 234712
Blocking: 644359
Blocking: 415586
Blocking: 644597
Blocking: 644600
Blocking: 648328
Blocking: 595784
Blocking: 649390
Blocking: -634946
Blockedon: 634946
Blocking: -634942
Blockedon: 634942
Blocking: 616325
Labels: Milestone-Afterglow
Labels: -OS-Linux
Blockedon: 661680
Blocking: 665147
Blocking: 678172
Blocking: 680659
Labels: -Milestone-Afterglow Milestone-Turndown
Blocking: 630366
Blocking: 409583
Blocking: 692667
Blocking: 698542
Blocking: 665222
Blocking: 709465
Blocking: 711598
Blocking: 713839
Blocking: 723607
Cc: -aga...@chromium.org
Owner: aga...@chromium.org
Blocking: 375734
List of repos still using Rietveld for review: https://paste.googleplex.com/4511361488912384
Project Member

Comment 102 by bugdroid1@chromium.org, Jun 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra/+/23e94e58923e5c2826817245134abc75a39f94d6

commit 23e94e58923e5c2826817245134abc75a39f94d6
Author: Aaron Gable <agable@chromium.org>
Date: Wed Jun 28 17:45:15 2017

Rietveld: refuse new uploads in read-only projects

This will help enforce that projects which are fully supported
on Gerrit don't have any lingering Rietveld usage, and help us
burn down the list of repos capable of using Rietveld until
none are left.

Eventually this will become a whitelist, and then empty.

Bug:  600469 ,  634946 
Change-Id: I472e57d431f54264a5aaf3e0aa09dbec2914f542
Reviewed-on: https://chromium-review.googlesource.com/550776
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Commit-Queue: Aaron Gable <agable@chromium.org>

[modify] https://crrev.com/23e94e58923e5c2826817245134abc75a39f94d6/appengine/chromium_rietveld/settings.py
[modify] https://crrev.com/23e94e58923e5c2826817245134abc75a39f94d6/appengine/chromium_rietveld/codereview/views.py

Project Member

Comment 103 by bugdroid1@chromium.org, Jun 29 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/depot_tools/+/665a4398b54fdee66c2c9d16c88823c7c55f2e6b

commit 665a4398b54fdee66c2c9d16c88823c7c55f2e6b
Author: Aaron Gable <agable@chromium.org>
Date: Thu Jun 29 18:33:00 2017

git-cl-upload: warn if no rietveld project specified

Bug:  600469 
Change-Id: I4ebc7b0f28204ae4a4649e51d6aac8fc2f43ed20
Reviewed-on: https://chromium-review.googlesource.com/556152
Reviewed-by: Andrew Bonventre <andybons@chromium.org>
Commit-Queue: Aaron Gable <agable@chromium.org>

[modify] https://crrev.com/665a4398b54fdee66c2c9d16c88823c7c55f2e6b/git_cl.py

Project Member

Comment 104 by bugdroid1@chromium.org, Jun 29 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/depot_tools/+/021372392977ab78a41c18e99897e64589385e7b

commit 021372392977ab78a41c18e99897e64589385e7b
Author: Aaron Gable <agable@chromium.org>
Date: Thu Jun 29 18:56:20 2017

Rietveld upload.py: send remote url instead of root hash

Using the remote url is much more useful than the hash of the
first commit in the repo, since we can use it to track down
repos, teams, and people still using Rietveld for review.

Bug:  600469 
Change-Id: I067594794ba77692c11daf63b5f5844d36209a08
Reviewed-on: https://chromium-review.googlesource.com/556301
Reviewed-by: Andrew Bonventre <andybons@chromium.org>
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Commit-Queue: Aaron Gable <agable@chromium.org>

[modify] https://crrev.com/021372392977ab78a41c18e99897e64589385e7b/third_party/upload.py

Blocking: 634942
Blockedon: -634942
Blockedon: 731091 685318 685321 726507 737149 738329
This is the list of repos which have had changes uploaded for review in the past year (since we turned off SVN):

{
  "https://chromium.googlesource.com/android_ndk": 1,
  "https://chromium.googlesource.com/angle/angle": 1,
  "https://chromium.googlesource.com/breakpad/breakpad/src": 3,
  "https://chromium.googlesource.com/chromium/buildtools": 74,
  "https://chromium.googlesource.com/chromium/cdm": 13,
  "https://chromium.googlesource.com/chromium/deps/flac": 2,
  "https://chromium.googlesource.com/chromium/deps/icu": 45,
  "https://chromium.googlesource.com/chromium/deps/libjpeg_turbo": 1,
  "https://chromium.googlesource.com/chromium/deps/libmtp": 3,
  "https://chromium.googlesource.com/chromium/deps/libsrtp": 10,
  "https://chromium.googlesource.com/chromium/deps/webrtc/webrtc.DEPS": 6,
  "https://chromium.googlesource.com/chromium/dom-distiller": 20,
  "https://chromium.googlesource.com/chromium/extensions-by-google": 6,
  "https://chromium.googlesource.com/chromium/pipa": 11,
  "https://chromium.googlesource.com/chromium/src": 82107,
  "https://chromium.googlesource.com/chromium/src/testing": 1,
  "https://chromium.googlesource.com/chromium/tools/build": 1272,
  "https://chromium.googlesource.com/chromium/tools/chrome-devtools-frontend": 7,
  "https://chromium.googlesource.com/chromium/tools/depot_tools": 270,
  "https://chromium.googlesource.com/chromiumos/drm_hwcomposer": 1,
  "https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay": 2,
  "https://chromium.googlesource.com/chromiumos/platform/drm-tests": 1,
  "https://chromium.googlesource.com/chromiumos/platform/minigbm": 1,
  "https://chromium.googlesource.com/chromiumos/platform/system_api": 1,
  "https://chromium.googlesource.com/chromiumos/platform2": 2,
  "https://chromium.googlesource.com/chromiumos/third_party/adhd": 1,
  "https://chromium.googlesource.com/chromiumos/third_party/leveldb": 3,
  "https://chromium.googlesource.com/chromiumos/third_party/tpm2": 2,
  "https://chromium.googlesource.com/dart/dartium/blink": 17,
  "https://chromium.googlesource.com/dart/dartium/src": 1,
  "https://chromium.googlesource.com/deps/inspector_protocol": 24,
  "https://chromium.googlesource.com/external/gyp": 5,
  "https://chromium.googlesource.com/external/linux-syscall-support/lss": 2,
  "https://chromium.googlesource.com/external/mojo": 4,
  "https://chromium.googlesource.com/external/netty-tcnative": 1,
  "https://chromium.googlesource.com/external/netty4": 1,
  "https://chromium.googlesource.com/external/py_trace_event": 1,
  "https://chromium.googlesource.com/external/robolectric": 2,
  "https://chromium.googlesource.com/external/webrtc": 6450,
  "https://chromium.googlesource.com/external/webrtc/deps/third_party/gtest-parallel": 4,
  "https://chromium.googlesource.com/external/webrtc/deps/third_party/openmax": 1,
  "https://chromium.googlesource.com/external/webrtc/deps/third_party/winsdk_samples_v71": 2,
  "https://chromium.googlesource.com/infra/infra": 1663,
  "https://chromium.googlesource.com/infra/infra_experimental": 7,
  "https://chromium.googlesource.com/infra/testing/expect_tests": 2,
  "https://chromium.googlesource.com/libyuv/libyuv": 78,
  "https://chromium.googlesource.com/native_client/pnacl-libcxx": 1,
  "https://chromium.googlesource.com/native_client/pnacl-subzero": 80,
  "https://chromium.googlesource.com/native_client/src/build_overrides": 3,
  "https://chromium.googlesource.com/native_client/src/native_client": 37,
  "https://chromium.googlesource.com/v8/v8": 6335,
  "https://chromium.googlesource.com/webm/libwebp": 1,
  "https://chromium.googlesource.com/webports": 2,
  "https://github.com/GoogleChrome/custom-tabs-client": 20,
  "https://github.com/catapult-project/catapult": 2320,
  "https://github.com/dart-lang/angular_analyzer_plugin": 3,
  "https://github.com/dart-lang/async": 8,
  "https://github.com/dart-lang/boringssl_gen": 1,
  "https://github.com/dart-lang/cli_util": 1,
  "https://github.com/dart-lang/code_transformers": 1,
  "https://github.com/dart-lang/collection": 10,
  "https://github.com/dart-lang/crypto": 1,
  "https://github.com/dart-lang/dart2js_info": 8,
  "https://github.com/dart-lang/dart_style": 11,
  "https://github.com/dart-lang/dartdoc": 1,
  "https://github.com/dart-lang/dev_compiler": 13,
  "https://github.com/dart-lang/fixnum": 2,
  "https://github.com/dart-lang/func": 1,
  "https://github.com/dart-lang/googleapis": 1,
  "https://github.com/dart-lang/html": 2,
  "https://github.com/dart-lang/http_multi_server": 1,
  "https://github.com/dart-lang/intl": 2,
  "https://github.com/dart-lang/linter": 40,
  "https://github.com/dart-lang/matcher": 2,
  "https://github.com/dart-lang/mime": 1,
  "https://github.com/dart-lang/observatory_pub_packages": 4,
  "https://github.com/dart-lang/observe": 1,
  "https://github.com/dart-lang/package_config": 4,
  "https://github.com/dart-lang/plugin": 1,
  "https://github.com/dart-lang/pool": 1,
  "https://github.com/dart-lang/protobuf": 4,
  "https://github.com/dart-lang/pub": 6,
  "https://github.com/dart-lang/rasta": 11,
  "https://github.com/dart-lang/reflectable": 2,
  "https://github.com/dart-lang/reify": 133,
  "https://github.com/dart-lang/resource": 7,
  "https://github.com/dart-lang/root_certificates": 1,
  "https://github.com/dart-lang/sdk": 7851,
  "https://github.com/dart-lang/shelf": 1,
  "https://github.com/dart-lang/smoke": 2,
  "https://github.com/dart-lang/source_map_stack_trace": 1,
  "https://github.com/dart-lang/source_maps": 6,
  "https://github.com/dart-lang/stack_trace": 2,
  "https://github.com/dart-lang/test": 10,
  "https://github.com/dart-lang/test_reflective_loader": 1,
  "https://github.com/dart-lang/tuple": 2,
  "https://github.com/dart-lang/webcore": 3,
  "https://github.com/dartino/sdk": 52,
  "https://github.com/google/googletest": 1,
  "https://github.com/google/syzygy": 107,
  "https://github.com/kripken/emscripten-fastcomp-clang": 2,
  "https://github.com/llvm-mirror/llvm": 3,
  "https://github.com/luci/gkvlite": 1,
  "https://github.com/luci/luci-gae": 49,
  "https://github.com/luci/luci-go": 837,
  "https://github.com/luci/luci-py": 718,
  "https://github.com/luci/recipes-py": 363,
  "https://github.com/peter-ahe-google/testing": 39,
  "https://pdfium.googlesource.com/pdfium": 774,
  "https://skia.googlesource.com/buildbot": 43,
  "https://skia.googlesource.com/skia": 896,
  "0483dcf0dabcc2fcca96ffc3c54c61abd0eaca9e": 1,
  "066fb3dae98e6ebdbd735ee46ccd3370c9407122": 1,
  "1c7f426c6bf2a75999adf4ad9e7fda57d6460c87": 5,
  "21019e4320de30b776093294bc7574073b265d93": 1,
  "21ef90f5993336371f1d9f556ab3fefb5426ca73": 1,
  "23c97acf0ee2ab777e35ccfb83660ab086cc1a0c": 1,
  "277394e4fcf5793915506401a071051971af6bed": 1,
  "2b23d1ca25b1152d5b8f6251e16ef4a20c082bb3": 1,
  "2c3d6d42bbc7a53da2779cae75b48f2d026ad607": 1,
  "30ba95f277dfe60d780b973c0b1682b4f1ba0d4f": 1,
  "32511afbb032865ce09a584f4e060a795690acde": 10,
  "388568d2ce8f5894950c05d5f4dab124ceb8b7eb": 2,
  "389da3c9427a6886846e905e818dc1d29508473b": 12,
  "3df4ebd528c0afb1459e1f9fc7b843e2dade6d74": 2,
  "47636c28b2b10840e767b260c18db8818b0d866f": 1,
  "4fc942258fe5509549333b9487ec018e3c8c5b10": 1,
  "522309d18f07418aeed5fe278c0446ca41bb668a": 1,
  "56b111c5b5560dbc19b1ba7c125e829733456213": 2,
  "56b544d579b329574557948ef0d5e0034a4f5be8": 1,
  "570baf7cfc0a190f8fb1dc9707b5ec4cf2496045": 9,
  "5a7b010196548eb6bfceff7429c1d15fea8352ac": 1,
  "5d75ec54d36642a1fdbb36da65ab84757accd9ef": 32,
  "64c83aae07ea983b1d790c12e340ff11e22d24b4": 1,
  "68394d5702cda02e381143b42f0e604bb71db4c8": 1,
  "6bba33ed19eda9db066c8c3ee2fa8f46256c79bd": 7,
  "705cd73c15d3031380be1bf711199c3e62953184": 3,
  "724ac40b8dbf3aaad74321f0abf8584ed28c611d": 1,
  "74ac2d20bc1bc02714e4f4f24ecf2b98fb598bff": 9,
  "79434047704f03902cbc15b9aa4e4bb78cd581ce": 1,
  "799c10ad316fbd80501ec41f7608166aa6e62760": 20,
  "7b0bbdbe66067295c4503eebde53b2ebf82e1764": 1,
  "84858683f5a914db00d30d036d72604254d61c58": 1,
  "853af7a9eaac85d3366a20a2eb76cc34d08afcfb": 1,
  "908b62fe1e81e4f06c6910a4885c2ba56b18b675": 3,
  "9428adde2645a694c9f9741a93360fe68f0d7048": 1,
  "9bd9eeba71904e9f30417811f2489c9698311bd1": 1,
  "a5570a457e513a66771407ebf0b6d7e8abe23a14": 7,
  "a7f9e752ca10eabb5dde430f7f7ada4b8292ef22": 1,
  "af938a15938a5c427e8ec0b6c96cdbe9aa57a9ad": 1,
  "b302dccaba1f4c7244a75325c66ee0f6cba28dac": 1,
  "b555b8a9c2e7b5fb5615fc422c905d35e602b258": 1,
  "b58f576ba586c18986315cb80e9b70fc8de5f71a": 1,
  "b926e423af4c25b232d785ac5295ce3189abb933": 1,
  "be0e5c097fc206b863ce9fe6b3cfd6974b0110f4": 2,
  "be2b864aad3d7767c83ea1fd30d1eb6efd118be8": 2,
  "bfb09453b6aeceba3614e87265059c44ef5142a6": 1,
  "c3d6f6c3f115f0a2adf57ba94041f889a2c8ff2b": 8,
  "d0e12dc78dade03496a03c91dd2b9a70a75d1e16": 37,
  "d34c4617ba875af2db3e54765eaab98c1e369ec1": 1,
  "dfe57d3292f9caa066ba658529314ed62a041b3f": 1,
  "e124e7ca3de99b1967b08bc8f90e8881a40c18d5": 6,
  "e1b4c66b4ffd54d7c7452ee1490b230506ee2cb9": 1,
  "e9830d2e13a0ab70e1201f731c2e23c4de71e120": 2,
  "eac9ccd53ff20e0f1fc0508278e148e6842cd48f": 2,
  "f805ce2a77007567bca9b3f7c5ae7509e423d52f": 1,
  "fde624e85deceb0590beeb5ab65a06f8e31a1ffa": 1,
}

Commentary:
+ Many of these (e.g. infra, skia, pdfium) have already been migrated to Gerrit
- Many of these are github repos, for which switching to Gerrit is rather non-trivial. We have other tracking bugs for those (now blocking this one)
- Many of these are unidentified -- we only have the "repo_guid" hash, which is *a* commit in the repo (the first if the uploader has a full clone; an arbitrary one if they have a shallow clone), and none of those hashes exist on chromium.googlesource.com. We could maybe identify them (github? other googlesource hosts?) with some more effort.
+ The remaining (not-already-migrated, not-github, positively-identified) ones are few and easy
Additional repos in this set which are surprise github mirrors:
chromium/dom-distiller
external/robolectric
external/py_trace_event
external/mojo

Repos which are surprise *manually maintained* github mirrors:
external/netty4
external/netty-tcnative

Of those, dom-distiller is the only one to have actual reviews on Rietveld, and it is being deprecated. The others either actively use github (netty, robolectric), have moved (mojo), or bypass review entirely (py_trace_event).
The only repos that are still configured to upload changes to Rietveld are those which are hosted on GitHub. Working on the mirroring solution for those (see blocking bugs) and then we can finish work on this.
Blocking: 751271
Blocking: 751756
Blockedon: 743191
The only repos still actively uploading changes to external Rietveld are:
https://github.com/dart-lang/*
https://github.com/google/syzygy
https://github.com/catapult-project/catapult
https://chromium.googlesource.com/external/webrtc

dart is tracked in  issue 737149 
syzygy has said they're going dormant and fine losing their code-review system
catapult is tracked in  issue 731091 
webrtc is tracked in  issue 738329 
Blockedon: 753912
syzygy is actually going to migrate as well, tracked in  issue 753912 
Blockedon: 749769
Blockedon: 766215
Blockedon: 672378
Project Member

Comment 119 by bugdroid1@chromium.org, Sep 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra/+/81bcdb8aafb7a21c6ea4a7bd2ea86951b6cdf8f8

commit 81bcdb8aafb7a21c6ea4a7bd2ea86951b6cdf8f8
Author: Aaron Gable <agable@chromium.org>
Date: Thu Sep 28 23:05:45 2017

Rietveld: Refuse uploads of new changes to all projects

R=iannucci@chromium.org

Bug:  600469 
Change-Id: I9141f04910159da7bde2ae6e411bbb508641f4f1
Reviewed-on: https://chromium-review.googlesource.com/691055
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Commit-Queue: Aaron Gable <agable@chromium.org>

[modify] https://crrev.com/81bcdb8aafb7a21c6ea4a7bd2ea86951b6cdf8f8/appengine/chromium_rietveld/settings.py
[modify] https://crrev.com/81bcdb8aafb7a21c6ea4a7bd2ea86951b6cdf8f8/appengine/chromium_rietveld/codereview/views.py

Project Member

Comment 120 by bugdroid1@chromium.org, Sep 28 2017

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infra/infra_internal/+/2c371be2c7539a276c445caf16275262aadda56b

commit 2c371be2c7539a276c445caf16275262aadda56b
Author: Aaron Gable <agable@chromium.org>
Date: Thu Sep 28 23:10:02 2017

Blocking: 770327
Status: Fixed (was: Assigned)
The king is dead, long live the king!

Followup API/UI work tracked in https://bugs.chromium.org/p/chromium/issues/detail?id=770327
All other work tracked in https://bugs.chromium.org/p/chromium/issues/list?can=2&q=Proj%3DGerrit-Migration+Milestone%3DTurndown
πŸŽ‰πŸŽ‰YAYπŸŽ‰πŸŽ‰
Blocking: 770592
Blocking: -492216
Showing comments 26 - 125 of 125 Older

Sign in to add a comment