Closed
Bug 1262548
Opened 8 years ago
Closed 8 years ago
Move all rbbz code into mozreview
Categories
(MozReview Graveyard :: Review Board: Extension, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: mcote, Assigned: davidwalsh)
Details
Attachments
(10 files, 16 obsolete files)
58 bytes,
text/x-review-board-request
|
smacleod
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
smacleod
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
smacleod
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
smacleod
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
smacleod
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
smacleod
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
smacleod
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
smacleod
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
smacleod
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
smacleod
:
review+
|
Details |
We originally tried to separate code along certain lines into multiple extensions. Currently they are in version-control-tools as pylib/rbbz and pylib/mozreview. This didn't really turn out to be useful, and now it's more-or-less random what is found in rbbz vs mozreview. We've since restricted new development to mozreview only, which occasionally necessitates moving code over from rbbz. We should just move everything over to mozreview now to make future development easier.
Comment 1•8 years ago
|
||
:davidwalsh, can you please move things over in logical units, using multiple commits, rather than using a single large commit that moves all the code. It will make it much easier to review when the move is finished :D
Assignee | ||
Comment 2•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/46109/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/46109/
Attachment #8740992 -
Flags: review?(smacleod)
Assignee | ||
Comment 3•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/46115/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/46115/
Attachment #8741009 -
Flags: review?(smacleod)
Assignee | ||
Comment 4•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/46167/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/46167/
Attachment #8741066 -
Flags: review?(smacleod)
Assignee | ||
Comment 5•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/46181/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/46181/
Attachment #8741081 -
Flags: review?(smacleod)
Assignee | ||
Comment 6•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/46203/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/46203/
Attachment #8741106 -
Flags: review?(smacleod)
Assignee | ||
Comment 7•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/46209/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/46209/
Attachment #8741123 -
Flags: review?(smacleod)
Assignee | ||
Comment 8•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/46507/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/46507/
Attachment #8741482 -
Flags: review?(smacleod)
Assignee | ||
Comment 9•8 years ago
|
||
https://reviewboard.mozilla.org/r/46507/#review43119 I need to break from this for the week but review can start; with this commit, however, I'm receiving the following error when I try to refresh: ``` $ ./mozreview refresh rbweb> PLAY [localhost] ************************************************************** rbweb> rbweb> GATHERING FACTS *************************************************************** hgrb> PLAY [localhost] ************************************************************** hgrb> GATHERING FACTS *************************************************************** bmoweb> httpd: stopped httpd: started rbweb> ok: [localhost]rbweb> TASK: [docker-rbweb | Build mozreview extension] ****************************** hgrb> ok: [localhost]hgrb> TASK: [hg-reviewboard | determine if running in Docker] *********************** hgrb> ok: [localhost] TASK: [hg-reviewboard | synchronize version-control-tools (Docker only)] ****** hgrb> changed: [localhost -> 127.0.0.1] PLAY RECAP ******************************************************************** localhost : ok=3 changed=1 unreachable=0 failed=0 hgrb> refreshed hgrb container successfully rbweb> failed: [localhost] => {"changed": true, "cmd": ["/venv/bin/python", "setup.py", "bdist_egg"], "delta": "0:00:02.320680", "end": "2016-04-14 19:32:15.957329", "rc": 1, "start": "2016-04-14 19:32:13.636649", "warnings": []} stderr: /venv/lib/python2.6/site-packages/setuptools/dist.py:285: UserWarning: Normalizing '0.1.2alpha0' to '0.1.2a0' normalized_version, /venv/lib/python2.6/site-packages/djblets/extensions/packaging.py:147: DeprecationWarning: Parameters to load are deprecated. Call .resolve and .require separately. extension = entrypoint.load(require=False) Traceback (most recent call last): File "setup.py", line 31, in <module> 'static/**/*.js' File "/venv/lib/python2.6/site-packages/reviewboard/extensions/packaging.py", line 48, in setup setuptools_setup(**setup_kwargs) File "/usr/lib64/python2.6/distutils/core.py", line 152, in setup dist.run_commands() File "/usr/lib64/python2.6/distutils/dist.py", line 975, in run_commands self.run_command(cmd) File "/usr/lib64/python2.6/distutils/dist.py", line 995, in run_command cmd_obj.run() File "/venv/lib/python2.6/site-packages/setuptools/command/bdist_egg.py", line 161, in run cmd = self.call_command('install_lib', warn_dir=0) File "/venv/lib/python2.6/site-packages/setuptools/command/bdist_egg.py", line 147, in call_command self.run_command(cmdname) File "/usr/lib64/python2.6/distutils/cmd.py", line 333, in run_command self.distribution.run_command(command) File "/usr/lib64/python2.6/distutils/dist.py", line 995, in run_command cmd_obj.run() File "/venv/lib/python2.6/site-packages/setuptools/command/install_lib.py", line 10, in run self.build() File "/usr/lib64/python2.6/distutils/command/install_lib.py", line 117, in build self.run_command('build_py') File "/usr/lib64/python2.6/distutils/cmd.py", line 333, in run_command self.distribution.run_command(command) File "/usr/lib64/python2.6/distutils/dist.py", line 995, in run_command cmd_obj.run() File "/venv/lib/python2.6/site-packages/djblets/extensions/packaging.py", line 283, in run self.run_command('build_static_files') File "/usr/lib64/python2.6/distutils/cmd.py", line 333, in run_command self.distribution.run_command(command) File "/usr/lib64/python2.6/distutils/dist.py", line 995, in run_command cmd_obj.run() File "/venv/lib/python2.6/site-packages/djblets/extensions/packaging.py", line 147, in run extension = entrypoint.load(require=False) File "/venv/lib/python2.6/site-packages/pkg_resources/__init__.py", line 2229, in load return self.resolve() File "/venv/lib/python2.6/site-packages/pkg_resources/__init__.py", line 2235, in resolve module = __import__(self.module_name, fromlist=['__name__'], level=0) File "/version-control-tools/pylib/mozreview/mozreview/extension.py", line 47, in <module> from mozreview.bugzilla.auth import BugzillaBackend File "/version-control-tools/pylib/mozreview/mozreview/bugzilla/auth.py", line 27, in <module> from mozreview.forms import BugzillaAuthSettingsForm File "/version-control-tools/pylib/mozreview/mozreview/forms.py", line 9, in <module> class MozReviewSettingsForm(SettingsForm): File "/version-control-tools/pylib/mozreview/mozreview/forms.py", line 13, in MozReviewSettingsForm mozreview.extension.SETTINGS_PATH) AttributeError: 'module' object has no attribute 'extension' stdout: running bdist_egg running egg_info writing requirements to mozreview.egg-info/requires.txt writing mozreview.egg-info/PKG-INFO writing top-level names to mozreview.egg-info/top_level.txt writing dependency_links to mozreview.egg-info/dependency_links.txt writing entry points to mozreview.egg-info/entry_points.txt reading manifest file 'mozreview.egg-info/SOURCES.txt' writing manifest file 'mozreview.egg-info/SOURCES.txt' installing library code to build/bdist.linux-x86_64/egg running install_lib running build_py running build_static_files FATAL: all hosts have already failed -- aborting PLAY RECAP ******************************************************************** to retry, use: --limit @/root/docker-rbweb.retry localhost : ok=1 changed=0 unreachable=0 failed=1 ``` The attribute is definitely there so the issue is probably something else, I just haven't been able to identify it yet.
Comment 10•8 years ago
|
||
Comment on attachment 8740992 [details] MozReview Request: MozReview: Migrate rbbz/forms.py to mozreview/forms.py (Bug 1262548). r?smacleod https://reviewboard.mozilla.org/r/46109/#review44193
Attachment #8740992 -
Flags: review?(smacleod) → review+
Comment 11•8 years ago
|
||
Comment on attachment 8741009 [details] MozReview Request: MozReview: Migrate rbbz/errors.py to mozreview/errors.py (Bug 1262548). r?smacleod https://reviewboard.mozilla.org/r/46115/#review44199 ::: pylib/mozreview/mozreview/errors.py:33 (Diff revision 1) > +class InvalidBugsError(PublishError): > + def __init__(self): > + PublishError.__init__(self, 'Exactly one bug ID must be provided.') > + Rather than migrate this we should just nuke it if it's unused.
Attachment #8741009 -
Flags: review?(smacleod)
Comment 12•8 years ago
|
||
Comment on attachment 8741066 [details] MozReview Request: MozReview: Migrate rbbz/middleware.py to mozreview/middleware.py (Bug 1262548). r?smacleod https://reviewboard.mozilla.org/r/46167/#review44201 ::: pylib/mozreview/mozreview/middleware.py:3 (Diff revision 1) > from mozreview.models import get_profile, MozReviewUserProfile > > +from django.conf import settings django imports should go in a block *before* any mozreview imports. Generally imports are grouped by: 1. Standard library 2. Third party imports 3. Local imports Since We're so heavily tied to reviewboard/django though you'll see we sometimes do: 1. Std lib 2. Third party excluding rb/django 3. reviewboard and django 4. local imports ::: pylib/rbbz/rbbz/extension.py:45 (Diff revision 1) > > logger = logging.getLogger(__name__) > > > class BugzillaExtension(Extension): > middleware = [CorsHeaderMiddleware] You'll want to migrate this over to mozreview, rather than just the middleware itself.
Attachment #8741066 -
Flags: review?(smacleod)
Comment 13•8 years ago
|
||
Comment on attachment 8741081 [details] MozReview Request: MozReview: Migrate rbbz/diffs.py to mozreview/diffs.py (Bug 1262548). r?smacleod https://reviewboard.mozilla.org/r/46181/#review44203
Attachment #8741081 -
Flags: review?(smacleod) → review+
Updated•8 years ago
|
Attachment #8741106 -
Flags: review?(smacleod)
Comment 14•8 years ago
|
||
Comment on attachment 8741106 [details] MozReview Request: MozReview: Migrate rbbz/auth.py to mozreview/bugzilla/auth.py (Bug 1262548). r?smacleod https://reviewboard.mozilla.org/r/46203/#review44205 ::: pylib/rbbz/rbbz/extension.py:52 (Diff revision 1) > resources = [ > bugzilla_cookie_login_resource, > ] > > def initialize(self): > AuthBackendHook(self, BugzillaBackend) This also needs to be migrated - which will require changes to the test environment and deployment scripts for setting up the auth backend. Without looking forward, you may or may not do this in a later commit, I'm unsure. Also, I'm unsure if you're running the test suite between all these changes, but the tests must pass after each commit, rather than just at the end, and you should check this as you go.
Updated•8 years ago
|
Attachment #8741123 -
Flags: review?(smacleod)
Comment 15•8 years ago
|
||
Comment on attachment 8741123 [details] MozReview Request: MozReview: Migrate rbbz/resources.py to mozreview/resources/bugzilla_login.py (Bug 1262548). r?smacleod https://reviewboard.mozilla.org/r/46209/#review44211 ::: pylib/rbbz/rbbz/extension.py:47 (Diff revision 1) > resources = [ > bugzilla_cookie_login_resource, > ] This needs to be migrated, as well as anything that makes requests to this endpoint since the URL is going to change. I *think* that we don't have any cookie login consumers left, but we need to make sure (they'd all be in the version-control-tools repository, so we don't have to worry about random third parties)
Comment 16•8 years ago
|
||
Comment on attachment 8741482 [details] MozReview Request: MozReview: Migrate rbbz/extension.py to mozreview/extension.py, cleanup rbbz artifacts (Bug 1262548). r?smacleod https://reviewboard.mozilla.org/r/46507/#review44213 A lot of the things migrated here should probably be migrated in the specific commit which moves the code for the feature, rather than treating extension.py as a unit itself. Also, we've lost a bunch of the documentation from rbbz files that were deleted as the contents weren't migrated to be part of mozreview. ::: ansible/roles/docker-rbweb/files/install-reviewboard:66 (Diff revision 1) > -site.run_manage_command('disable-extension', > ['rbmotd.extension.MotdExtension']) > > print('enabling extensions') > site.run_manage_command('enable-extension', ['mozreview.extension.MozReviewExtension']) > -site.run_manage_command('enable-extension', ['rbbz.extension.BugzillaExtension']) > +site.run_manage_command('enable-extension', ['mozreview.extension.BugzillaExtension']) This extension shouldn't (and doesn't that I can see) exist. All of its functionality should be rolled into the MozReviewExtension rather than having two Extensions in the extension.py - I think you've done this though, so nuke this stuff. ::: docs/hacking-mozreview.rst (Diff revision 1) > Bugzilla. > > Code Locations > ============== > > -``pylib/rbbz`` contains the modifications to Review Board to enable The documentation for `docs/rbbz/rbbz.rst` was completely deleted - we need to migrate its contents somewhere since the code all still exists. ::: pylib/mozreview/mozreview/bugzilla/errors.py:20 (Diff revision 1) > - BugzillaError.__init__(self, 'No Bugzilla URL provided in rbbz ' > + BugzillaError.__init__(self, 'No Bugzilla URL provided in mozreview ' > 'configuration.') I feel like this would *have* to have test changes, which I don't see in this patch? We need to make sure all the tests are passing. ::: pylib/mozreview/mozreview/extension.py:10 (Diff revision 1) > +from djblets.siteconfig.models import SiteConfiguration > > from reviewboard.diffviewer.opcode_generator import ( djblets imports dont need a space between them and reviewboard ::: pylib/mozreview/mozreview/extension.py:115 (Diff revision 1) > from mozreview.signal_handlers import ( > initialize_signal_handlers, > ) All your SignalHooks should go in here. ::: pylib/mozreview/mozreview/extension.py:191 (Diff revision 1) > autoland_request_update_resource, > autoland_trigger_resource, > batch_review_request_resource, > batch_review_resource, > bugzilla_api_key_login_resource, > + bugzilla_cookie_login_resource, We need to make sure any urls referencing this resource have been updated. ::: pylib/mozreview/mozreview/extension.py:327 (Diff revision 1) > # Instantiate the various signal handlers > initialize_signal_handlers(self) > > HostingServiceHook(self, HMORepository) > > + AuthBackendHook(self, BugzillaBackend) The configuration code for production and the test environment need to be updated for this change. We might have to come up with a migration plan for the actual deployments. ::: pylib/mozreview/mozreview/extension.py:379 (Diff revision 1) > +def get_reply_url(reply, site=None, siteconfig=None): > + """ Get the URL for a reply to a review. > + > + Since replies can have multiple comments, we can't link to a specific > + comment, so we link to the parent review which the reply is targeted at. > + """ > + return get_obj_url(reply.base_reply_to, site=site, siteconfig=siteconfig) I *think* mozreview has its own version of this somewhere which should be used rather than having two copies. ::: pylib/mozreview/mozreview/extension.py:389 (Diff revision 1) > + """ > + return get_obj_url(reply.base_reply_to, site=site, siteconfig=siteconfig) > + > + > +@bugzilla_to_publish_errors > +def on_review_publishing(user, review, **kwargs): This signal belongs in the signal_handlers.py ::: pylib/mozreview/mozreview/extension.py:443 (Diff revision 1) > + if comment and not commented: > + b.post_comment(bug_id, comment) > + > + > +@bugzilla_to_publish_errors > +def on_reply_publishing(user, reply, **kwargs): signal_handlers.py
Attachment #8741482 -
Flags: review?(smacleod)
Assignee | ||
Comment 17•8 years ago
|
||
https://reviewboard.mozilla.org/r/46203/#review44205 > This also needs to be migrated - which will require changes to the test environment and deployment scripts for setting up the auth backend. Without looking forward, you may or may not do this in a later commit, I'm unsure. > > Also, I'm unsure if you're running the test suite between all these changes, but the tests must pass after each commit, rather than just at the end, and you should check this as you go. Tests haven't been working locally for myself or others, as I mentioned. VM issues prevented me from running them.
Assignee | ||
Comment 18•8 years ago
|
||
Comment on attachment 8741009 [details] MozReview Request: MozReview: Migrate rbbz/errors.py to mozreview/errors.py (Bug 1262548). r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/46115/diff/1-2/
Attachment #8741009 -
Flags: review?(smacleod)
Attachment #8741066 -
Flags: review?(smacleod)
Attachment #8741106 -
Flags: review?(smacleod)
Attachment #8741123 -
Flags: review?(smacleod)
Attachment #8741482 -
Flags: review?(smacleod)
Assignee | ||
Comment 19•8 years ago
|
||
Comment on attachment 8741066 [details] MozReview Request: MozReview: Migrate rbbz/middleware.py to mozreview/middleware.py (Bug 1262548). r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/46167/diff/1-2/
Assignee | ||
Comment 20•8 years ago
|
||
Comment on attachment 8741081 [details] MozReview Request: MozReview: Migrate rbbz/diffs.py to mozreview/diffs.py (Bug 1262548). r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/46181/diff/1-2/
Assignee | ||
Comment 21•8 years ago
|
||
Comment on attachment 8741106 [details] MozReview Request: MozReview: Migrate rbbz/auth.py to mozreview/bugzilla/auth.py (Bug 1262548). r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/46203/diff/1-2/
Assignee | ||
Comment 22•8 years ago
|
||
Comment on attachment 8741123 [details] MozReview Request: MozReview: Migrate rbbz/resources.py to mozreview/resources/bugzilla_login.py (Bug 1262548). r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/46209/diff/1-2/
Assignee | ||
Comment 23•8 years ago
|
||
Comment on attachment 8741482 [details] MozReview Request: MozReview: Migrate rbbz/extension.py to mozreview/extension.py, cleanup rbbz artifacts (Bug 1262548). r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/46507/diff/1-2/
Assignee | ||
Comment 24•8 years ago
|
||
https://reviewboard.mozilla.org/r/46167/#review44201 > You'll want to migrate this over to mozreview, rather than just the middleware itself. By "this" do you mean you'd like to see the extension moved in this commit as well? (instead of here: https://reviewboard.mozilla.org/r/46507/ ?)
Comment 25•8 years ago
|
||
https://reviewboard.mozilla.org/r/46167/#review44201 > By "this" do you mean you'd like to see the extension moved in this commit as well? (instead of here: https://reviewboard.mozilla.org/r/46507/ ?) Not the whole extension itself, just this piece of it. So you'd remove `CorsHeaderMiddleware` from `BugzillaExtension.middleware` and instead have it be part of `MozReviewExtension`.
Assignee | ||
Comment 26•8 years ago
|
||
Comment on attachment 8741066 [details] MozReview Request: MozReview: Migrate rbbz/middleware.py to mozreview/middleware.py (Bug 1262548). r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/46167/diff/2-3/
Assignee | ||
Comment 27•8 years ago
|
||
Comment on attachment 8741081 [details] MozReview Request: MozReview: Migrate rbbz/diffs.py to mozreview/diffs.py (Bug 1262548). r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/46181/diff/2-3/
Assignee | ||
Comment 28•8 years ago
|
||
Comment on attachment 8741106 [details] MozReview Request: MozReview: Migrate rbbz/auth.py to mozreview/bugzilla/auth.py (Bug 1262548). r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/46203/diff/2-3/
Assignee | ||
Comment 29•8 years ago
|
||
Comment on attachment 8741123 [details] MozReview Request: MozReview: Migrate rbbz/resources.py to mozreview/resources/bugzilla_login.py (Bug 1262548). r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/46209/diff/2-3/
Assignee | ||
Comment 30•8 years ago
|
||
Comment on attachment 8741482 [details] MozReview Request: MozReview: Migrate rbbz/extension.py to mozreview/extension.py, cleanup rbbz artifacts (Bug 1262548). r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/46507/diff/2-3/
Assignee | ||
Updated•8 years ago
|
Attachment #8740992 -
Attachment is obsolete: true
Assignee | ||
Updated•8 years ago
|
Attachment #8741009 -
Attachment is obsolete: true
Attachment #8741009 -
Flags: review?(smacleod)
Assignee | ||
Comment 31•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/48183/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/48183/
Attachment #8743912 -
Flags: review?(smacleod)
Assignee | ||
Updated•8 years ago
|
Attachment #8741066 -
Attachment is obsolete: true
Attachment #8741066 -
Flags: review?(smacleod)
Assignee | ||
Updated•8 years ago
|
Attachment #8741081 -
Attachment is obsolete: true
Assignee | ||
Updated•8 years ago
|
Attachment #8741106 -
Attachment is obsolete: true
Attachment #8741106 -
Flags: review?(smacleod)
Assignee | ||
Updated•8 years ago
|
Attachment #8741123 -
Attachment is obsolete: true
Attachment #8741123 -
Flags: review?(smacleod)
Assignee | ||
Updated•8 years ago
|
Attachment #8741482 -
Attachment is obsolete: true
Attachment #8741482 -
Flags: review?(smacleod)
Assignee | ||
Comment 32•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/48185/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/48185/
Attachment #8743915 -
Flags: review?(smacleod)
Assignee | ||
Updated•8 years ago
|
Attachment #8743912 -
Attachment is obsolete: true
Attachment #8743912 -
Flags: review?(smacleod)
Assignee | ||
Comment 33•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/48187/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/48187/
Attachment #8743919 -
Flags: review?(smacleod)
Attachment #8743920 -
Flags: review?(smacleod)
Attachment #8743921 -
Flags: review?(smacleod)
Attachment #8743922 -
Flags: review?(smacleod)
Assignee | ||
Comment 34•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/48189/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/48189/
Assignee | ||
Comment 35•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/48191/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/48191/
Assignee | ||
Comment 36•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/48193/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/48193/
Assignee | ||
Updated•8 years ago
|
Attachment #8743915 -
Attachment is obsolete: true
Attachment #8743915 -
Flags: review?(smacleod)
Assignee | ||
Updated•8 years ago
|
Attachment #8743919 -
Attachment is obsolete: true
Attachment #8743919 -
Flags: review?(smacleod)
Assignee | ||
Updated•8 years ago
|
Attachment #8743920 -
Attachment is obsolete: true
Attachment #8743920 -
Flags: review?(smacleod)
Assignee | ||
Updated•8 years ago
|
Attachment #8743921 -
Attachment is obsolete: true
Attachment #8743921 -
Flags: review?(smacleod)
Assignee | ||
Updated•8 years ago
|
Attachment #8743922 -
Attachment is obsolete: true
Attachment #8743922 -
Flags: review?(smacleod)
Assignee | ||
Comment 37•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/49759/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/49759/
Attachment #8747235 -
Flags: review?(smacleod)
Attachment #8747236 -
Flags: review?(smacleod)
Attachment #8747237 -
Flags: review?(smacleod)
Attachment #8747238 -
Flags: review?(smacleod)
Attachment #8747239 -
Flags: review?(smacleod)
Attachment #8747240 -
Flags: review?(smacleod)
Attachment #8747241 -
Flags: review?(smacleod)
Assignee | ||
Comment 38•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/49761/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/49761/
Assignee | ||
Comment 39•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/49763/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/49763/
Assignee | ||
Comment 40•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/49765/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/49765/
Assignee | ||
Comment 41•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/49767/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/49767/
Assignee | ||
Comment 42•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/49769/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/49769/
Assignee | ||
Comment 43•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/49771/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/49771/
Assignee | ||
Comment 44•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/49773/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/49773/
Assignee | ||
Comment 45•8 years ago
|
||
https://reviewboard.mozilla.org/r/49771/#review46539 We'll need to decide how and where we want to integrate the rbbz documentation. There's also a `docs/rbbz/rbbz.rst` file we'll need to decide what to do with. This commit acts as a placeholder for all documentation updates.
Assignee | ||
Comment 46•8 years ago
|
||
https://reviewboard.mozilla.org/r/49773/#review46541 No `r?` request on this as it introduces 3 test failures (all commits prior to this last one pass with no test failures) Any advice in finally removing rbbz would be appreciated. Additionally the previous review mentioned possible need for deploy migrations -- we'll need to decide if that's necessary and in what capacity.
Assignee | ||
Comment 47•8 years ago
|
||
https://reviewboard.mozilla.org/r/49759/#review46543
Comment 48•8 years ago
|
||
Comment on attachment 8747235 [details] MozReview: Remove rbbz/errors.py (Bug 1262548). https://reviewboard.mozilla.org/r/49759/#review51788 The commit message for this should really be changed to indicate we're just removing a file with unused code rather than migrating it.
Attachment #8747235 -
Flags: review?(smacleod) → review+
Comment 49•8 years ago
|
||
Comment on attachment 8747236 [details] MozReview: Migrate rbbz/diffs.py to mozreview/diffs.py (Bug 1262548). https://reviewboard.mozilla.org/r/49761/#review51790
Attachment #8747236 -
Flags: review?(smacleod) → review+
Comment 50•8 years ago
|
||
Comment on attachment 8747237 [details] MozReview: Migrate rbbz/forms.py to mozreview/forms.py (Bug 1262548). https://reviewboard.mozilla.org/r/49763/#review51792 ::: pylib/mozreview/mozreview/forms.py:18 (Diff revision 1) > mozreview.extension.SETTINGS_PATH) > ) > + > + > +class BugzillaAuthSettingsForm(SiteSettingsForm): > + Nuke this blank line as part of the move.
Attachment #8747237 -
Flags: review?(smacleod) → review+
Updated•8 years ago
|
Attachment #8747238 -
Flags: review?(smacleod) → review+
Comment 51•8 years ago
|
||
Comment on attachment 8747238 [details] MozReview: Migrate rbbz/middleware.py to mozreview/middleware.py (Bug 1262548). https://reviewboard.mozilla.org/r/49765/#review51802 ::: pylib/rbbz/rbbz/extension.py:30 (Diff revision 1) > ) > from mozreview.extra_data import ( > is_parent, > is_pushed, > ) > +from mozreview.middleware import CorsHeaderMiddleware unused import
Comment 52•8 years ago
|
||
Comment on attachment 8747239 [details] MozReview: Remove rbbz/resources (BugzillaCookieLoginResource) as it doesn't appear to be used (Bug 1262548). https://reviewboard.mozilla.org/r/49767/#review51804 ::: pylib/rbbz/rbbz/extension.py (Diff revision 1) > logger = logging.getLogger(__name__) > > > class BugzillaExtension(Extension): > - resources = [ > - bugzilla_cookie_login_resource, Can you mention that it's removing the BugzillaCookieLoginResource in the commit message so that it's greppable.
Attachment #8747239 -
Flags: review?(smacleod) → review+
Comment 53•8 years ago
|
||
Comment on attachment 8747240 [details] MozReview: Move rbbz/auth to mozreview extension (Bug 1262548). https://reviewboard.mozilla.org/r/49769/#review51810
Attachment #8747240 -
Flags: review?(smacleod) → review+
Comment 54•8 years ago
|
||
Comment on attachment 8747241 [details] MozReview: Remove rbbz documentation (Bug 1262548). https://reviewboard.mozilla.org/r/49771/#review51812 Ah, so I totally missed something. check out `docs/rbbz/*`, we should probably be modifying this in each commit removing the relvant parts. This folder would probably make a much better location for this documentation too, but we really should integrate it better.
Attachment #8747241 -
Flags: review?(smacleod)
Comment 55•8 years ago
|
||
https://reviewboard.mozilla.org/r/49771/#review46539 heh, I really should have read this before making my review. Most of `docs/rbbz/rbbz.rst` will be broken since we've moved the location of the python classes it references... I also don't think it's that important to maintain that type of documentation in there. As for where to stick the other stuff, I'd say either in the hacking mozreview guide, in docstrings on classes such as the AuthBackend, or in our deployment guide on mana.
Comment 56•8 years ago
|
||
https://reviewboard.mozilla.org/r/49773/#review51826 Okay, so for the broken tests I'm not 100% sure what's going on, so a few questions: 1. Are the tests broken with a fresh rebuild at this commit, an incremental build from the previous commit, or both? 2. What is the difference in the test output? 3. Are they still broken if you leave an rbbz extension class with has `pass` as the body? ::: pylib/mozreview/mozreview/signal_handlers.py:132 (Diff revision 1) > + # Any abortable signal hooks that talk to Bugzilla should have > + # sandbox_errors=False, since we don't want to complete the action if > + # updating Bugzilla failed for any reason. This comment already exists in the docstring for this function. ::: pylib/mozreview/mozreview/signal_handlers.py:571 (Diff revision 1) > +@bugzilla_to_publish_errors > +def on_review_publishing(user, review, **kwargs): > + """Comment in the bug and potentially r+ or clear a review flag. > + > + Note that a reviewer *must* have editbugs to set an attachment flag on > + someone else's attachment (i.e. the standard BMO review process). > + > + TODO: Report lack-of-editbugs properly; see bug 1119065. > + """ > + review_request = review.review_request > + logger.info('Publishing review for user: %s review id: %s ' > + 'review request id: %s' % (user, review.id, > + review_request.id)) > + Can you please take care of these signals one by one each in their own commit, it makes it *much* easier for me to review.
Assignee | ||
Comment 57•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/55554/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/55554/
Attachment #8757032 -
Flags: review?(smacleod)
Attachment #8757033 -
Flags: review?(smacleod)
Attachment #8757034 -
Flags: review?(smacleod)
Attachment #8747241 -
Flags: review?(smacleod)
Assignee | ||
Comment 58•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/55556/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/55556/
Assignee | ||
Comment 59•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/55558/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/55558/
Assignee | ||
Comment 60•8 years ago
|
||
Comment on attachment 8747235 [details] MozReview: Remove rbbz/errors.py (Bug 1262548). Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49759/diff/1-2/
Assignee | ||
Comment 61•8 years ago
|
||
Comment on attachment 8747236 [details] MozReview: Migrate rbbz/diffs.py to mozreview/diffs.py (Bug 1262548). Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49761/diff/1-2/
Assignee | ||
Comment 62•8 years ago
|
||
Comment on attachment 8747237 [details] MozReview: Migrate rbbz/forms.py to mozreview/forms.py (Bug 1262548). Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49763/diff/1-2/
Assignee | ||
Comment 63•8 years ago
|
||
Comment on attachment 8747238 [details] MozReview: Migrate rbbz/middleware.py to mozreview/middleware.py (Bug 1262548). Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49765/diff/1-2/
Assignee | ||
Comment 64•8 years ago
|
||
Comment on attachment 8747239 [details] MozReview: Remove rbbz/resources (BugzillaCookieLoginResource) as it doesn't appear to be used (Bug 1262548). Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49767/diff/1-2/
Assignee | ||
Comment 65•8 years ago
|
||
Comment on attachment 8747240 [details] MozReview: Move rbbz/auth to mozreview extension (Bug 1262548). Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49769/diff/1-2/
Assignee | ||
Comment 66•8 years ago
|
||
Comment on attachment 8747241 [details] MozReview: Remove rbbz documentation (Bug 1262548). Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49771/diff/1-2/
Assignee | ||
Updated•8 years ago
|
Attachment #8747242 -
Attachment is obsolete: true
Assignee | ||
Comment 67•8 years ago
|
||
Comment on attachment 8747237 [details] MozReview: Migrate rbbz/forms.py to mozreview/forms.py (Bug 1262548). Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49763/diff/2-3/
Assignee | ||
Comment 68•8 years ago
|
||
Comment on attachment 8747238 [details] MozReview: Migrate rbbz/middleware.py to mozreview/middleware.py (Bug 1262548). Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49765/diff/2-3/
Assignee | ||
Comment 69•8 years ago
|
||
Comment on attachment 8747239 [details] MozReview: Remove rbbz/resources (BugzillaCookieLoginResource) as it doesn't appear to be used (Bug 1262548). Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49767/diff/2-3/
Assignee | ||
Comment 70•8 years ago
|
||
Comment on attachment 8747240 [details] MozReview: Move rbbz/auth to mozreview extension (Bug 1262548). Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49769/diff/2-3/
Assignee | ||
Comment 71•8 years ago
|
||
Comment on attachment 8747241 [details] MozReview: Remove rbbz documentation (Bug 1262548). Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49771/diff/2-3/
Assignee | ||
Comment 72•8 years ago
|
||
Comment on attachment 8757032 [details] MozReview: Move 'on_review_publishing' signal to mozreview (Bug 1262548). Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55554/diff/1-2/
Assignee | ||
Updated•8 years ago
|
Attachment #8757033 -
Attachment is obsolete: true
Attachment #8757033 -
Flags: review?(smacleod)
Assignee | ||
Updated•8 years ago
|
Attachment #8757034 -
Attachment is obsolete: true
Attachment #8757034 -
Flags: review?(smacleod)
Assignee | ||
Comment 73•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/55820/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/55820/
Attachment #8757344 -
Flags: review?(smacleod)
Attachment #8757345 -
Flags: review?(smacleod)
Assignee | ||
Comment 74•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/55822/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/55822/
Assignee | ||
Comment 75•8 years ago
|
||
Comment on attachment 8747239 [details] MozReview: Remove rbbz/resources (BugzillaCookieLoginResource) as it doesn't appear to be used (Bug 1262548). Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49767/diff/3-4/
Attachment #8747239 -
Attachment description: MozReview Request: MozReview: Remove rbbz/resources as it doesn't appear to be used (Bug 1262548). r?smacleod → MozReview Request: MozReview: Remove rbbz/resources (BugzillaCookieLoginResource) as it doesn't appear to be used (Bug 1262548). r?smacleod
Assignee | ||
Comment 76•8 years ago
|
||
Comment on attachment 8747240 [details] MozReview: Move rbbz/auth to mozreview extension (Bug 1262548). Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49769/diff/3-4/
Assignee | ||
Comment 77•8 years ago
|
||
Comment on attachment 8747241 [details] MozReview: Remove rbbz documentation (Bug 1262548). Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49771/diff/3-4/
Assignee | ||
Comment 78•8 years ago
|
||
Comment on attachment 8757032 [details] MozReview: Move 'on_review_publishing' signal to mozreview (Bug 1262548). Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55554/diff/2-3/
Assignee | ||
Comment 79•8 years ago
|
||
Comment on attachment 8757344 [details] MozReview: Move 'on_reply_publishing' signal to mozreview (Bug 1262548). Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55820/diff/1-2/
Assignee | ||
Comment 80•8 years ago
|
||
Comment on attachment 8757345 [details] MozReview: Remove rbbz extension and artifacts (Bug 1262548). Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55822/diff/1-2/
Assignee | ||
Comment 81•8 years ago
|
||
https://reviewboard.mozilla.org/r/49771/#review52496 What do we want to do with the rbbz-readme.md content? I'm not seeing a logical place to shift this content into and I don't have much insight as to what of this documentation is important anymore.
Assignee | ||
Comment 82•8 years ago
|
||
Comment on attachment 8757345 [details] MozReview: Remove rbbz extension and artifacts (Bug 1262548). Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55822/diff/2-3/
Reporter | ||
Comment 83•8 years ago
|
||
https://reviewboard.mozilla.org/r/49771/#review52496 I think we can ditch that. Most of it is from way back when we thought this could be a generic solution for other Bugzilla instances to use.
Assignee | ||
Comment 84•8 years ago
|
||
Comment on attachment 8747241 [details] MozReview: Remove rbbz documentation (Bug 1262548). Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49771/diff/4-5/
Assignee | ||
Comment 85•8 years ago
|
||
Comment on attachment 8757032 [details] MozReview: Move 'on_review_publishing' signal to mozreview (Bug 1262548). Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55554/diff/3-4/
Assignee | ||
Comment 86•8 years ago
|
||
Comment on attachment 8757344 [details] MozReview: Move 'on_reply_publishing' signal to mozreview (Bug 1262548). Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55820/diff/2-3/
Assignee | ||
Comment 87•8 years ago
|
||
Comment on attachment 8757345 [details] MozReview: Remove rbbz extension and artifacts (Bug 1262548). Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55822/diff/3-4/
Assignee | ||
Comment 88•8 years ago
|
||
https://reviewboard.mozilla.org/r/49757/#review52622 This should be good for another look!
Comment 89•8 years ago
|
||
https://reviewboard.mozilla.org/r/49759/#review55648 The commit message still needs to be changed, I won't `r-`, but please make sure to update it before landing.
Comment 90•8 years ago
|
||
Comment on attachment 8747241 [details] MozReview: Remove rbbz documentation (Bug 1262548). https://reviewboard.mozilla.org/r/49771/#review55650 Commit message no longer applicable.
Attachment #8747241 -
Flags: review?(smacleod) → review-
Comment 91•8 years ago
|
||
Comment on attachment 8757032 [details] MozReview: Move 'on_review_publishing' signal to mozreview (Bug 1262548). https://reviewboard.mozilla.org/r/55554/#review55652 ::: pylib/mozreview/mozreview/signal_handlers.py:631 (Diff revision 4) > + # TODO: Update all attachments in one call. This is not possible right > + # now because we have to potentially mix changing and creating flags. Hmm, this looks to be some left over cruft, we only update a single attachment when a review is publishing... Might as well nuke this.
Attachment #8757032 -
Flags: review?(smacleod) → review+
Updated•8 years ago
|
Attachment #8757344 -
Flags: review?(smacleod) → review+
Comment 92•8 years ago
|
||
Comment on attachment 8757344 [details] MozReview: Move 'on_reply_publishing' signal to mozreview (Bug 1262548). https://reviewboard.mozilla.org/r/55820/#review55654
Comment 93•8 years ago
|
||
Comment on attachment 8757345 [details] MozReview: Remove rbbz extension and artifacts (Bug 1262548). https://reviewboard.mozilla.org/r/55822/#review55682 ::: ansible/roles/docker-rbweb/files/install-reviewboard (Diff revision 4) > - ['rbbz.extension.BugzillaExtension']) > -site.run_manage_command('disable-extension', (This isn't necessarily a code issue you need to fix, just something I must explicitly call out). Deployment for this change could get a little interesting - what I believe might happen if we deploy as we normally do is the following: - We'll end up with all of the `rbbz` functionality inside of `MozReview`. - The old `rbbz` package will still be kicking around, with all of the duplicated functionality. - The old rbbz extension information will be kicking around in the database. - The old rbbz extension will be enabled. I'm not exactly sure how all of this will interact, or if we can get away with just turning rbbz off / uninstalling it manually after the deploy. The best bet though would be to write a new playbook which will do this all automatically.
Attachment #8757345 -
Flags: review?(smacleod) → review+
Assignee | ||
Comment 94•8 years ago
|
||
Comment on attachment 8747235 [details] MozReview: Remove rbbz/errors.py (Bug 1262548). Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49759/diff/2-3/
Attachment #8747235 -
Attachment description: MozReview Request: MozReview: Migrate rbbz/errors.py to mozreview/errors.py (Bug 1262548). r?smacleod → MozReview: Remove rbbz/errors.py (Bug 1262548).
Attachment #8747236 -
Attachment description: MozReview Request: MozReview: Migrate rbbz/diffs.py to mozreview/diffs.py (Bug 1262548). r?smacleod → MozReview: Migrate rbbz/diffs.py to mozreview/diffs.py (Bug 1262548).
Attachment #8747237 -
Attachment description: MozReview Request: MozReview: Migrate rbbz/forms.py to mozreview/forms.py (Bug 1262548). r?smacleod → MozReview: Migrate rbbz/forms.py to mozreview/forms.py (Bug 1262548).
Attachment #8747238 -
Attachment description: MozReview Request: MozReview: Migrate rbbz/middleware.py to mozreview/middleware.py (Bug 1262548). r?smacleod → MozReview: Migrate rbbz/middleware.py to mozreview/middleware.py (Bug 1262548).
Attachment #8747239 -
Attachment description: MozReview Request: MozReview: Remove rbbz/resources (BugzillaCookieLoginResource) as it doesn't appear to be used (Bug 1262548). r?smacleod → MozReview: Remove rbbz/resources (BugzillaCookieLoginResource) as it doesn't appear to be used (Bug 1262548).
Attachment #8747240 -
Attachment description: MozReview Request: MozReview: Move rbbz/auth to mozreview extension (Bug 1262548). r?smacleod → MozReview: Move rbbz/auth to mozreview extension (Bug 1262548).
Attachment #8747241 -
Attachment description: MozReview Request: MozReview: Placeholder documentation update (Bug 1262548). r?smacleod → MozReview: Remove rbbz documentation (Bug 1262548).
Attachment #8757032 -
Attachment description: MozReview Request: MozReview: Move 'on_review_publishing' signal to mozreview (Bug 1262548). r?smacleod → MozReview: Move 'on_review_publishing' signal to mozreview (Bug 1262548).
Attachment #8757344 -
Attachment description: MozReview Request: MozReview: Move 'on_reply_publishing' signal to mozreview (Bug 1262548). r?smacleod → MozReview: Move 'on_reply_publishing' signal to mozreview (Bug 1262548).
Attachment #8757345 -
Attachment description: MozReview Request: MozReview: Remove rbbz extension and artifacts (Bug 1262548). r?smacleod → MozReview: Remove rbbz extension and artifacts (Bug 1262548).
Attachment #8747241 -
Flags: review- → review?(smacleod)
Assignee | ||
Comment 95•8 years ago
|
||
Comment on attachment 8747236 [details] MozReview: Migrate rbbz/diffs.py to mozreview/diffs.py (Bug 1262548). Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49761/diff/2-3/
Assignee | ||
Comment 96•8 years ago
|
||
Comment on attachment 8747237 [details] MozReview: Migrate rbbz/forms.py to mozreview/forms.py (Bug 1262548). Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49763/diff/3-4/
Assignee | ||
Comment 97•8 years ago
|
||
Comment on attachment 8747238 [details] MozReview: Migrate rbbz/middleware.py to mozreview/middleware.py (Bug 1262548). Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49765/diff/3-4/
Assignee | ||
Comment 98•8 years ago
|
||
Comment on attachment 8747239 [details] MozReview: Remove rbbz/resources (BugzillaCookieLoginResource) as it doesn't appear to be used (Bug 1262548). Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49767/diff/4-5/
Assignee | ||
Comment 99•8 years ago
|
||
Comment on attachment 8747240 [details] MozReview: Move rbbz/auth to mozreview extension (Bug 1262548). Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49769/diff/4-5/
Assignee | ||
Comment 100•8 years ago
|
||
Comment on attachment 8747241 [details] MozReview: Remove rbbz documentation (Bug 1262548). Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49771/diff/5-6/
Assignee | ||
Comment 101•8 years ago
|
||
Comment on attachment 8757032 [details] MozReview: Move 'on_review_publishing' signal to mozreview (Bug 1262548). Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55554/diff/4-5/
Assignee | ||
Comment 102•8 years ago
|
||
Comment on attachment 8757344 [details] MozReview: Move 'on_reply_publishing' signal to mozreview (Bug 1262548). Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55820/diff/3-4/
Assignee | ||
Comment 103•8 years ago
|
||
Comment on attachment 8757345 [details] MozReview: Remove rbbz extension and artifacts (Bug 1262548). Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55822/diff/4-5/
Assignee | ||
Comment 104•8 years ago
|
||
https://reviewboard.mozilla.org/r/49771/#review55710 Updated the commit message!
Updated•8 years ago
|
Attachment #8747241 -
Flags: review?(smacleod) → review+
Comment 105•8 years ago
|
||
Comment on attachment 8747241 [details] MozReview: Remove rbbz documentation (Bug 1262548). https://reviewboard.mozilla.org/r/49771/#review55716
Assignee | ||
Comment 106•8 years ago
|
||
https://reviewboard.mozilla.org/r/55822/#review55946 I re-ran tests and everything passed except for 2 timeouts, but I think it's my environment because another run did fine. Really would like someone to run tests also so I'm not crazy.
Comment 107•8 years ago
|
||
(In reply to David Walsh :davidwalsh from comment #106) > I re-ran tests and everything passed except for 2 timeouts, but I think it's > my environment because another run did fine. Really would like someone to > run tests also so I'm not crazy. after fixing merge conflicts the tests ran here without issue.
Comment 108•8 years ago
|
||
the docs still refer to the rbbz extension, and need to be updated.
Flags: documentation?
Comment 109•8 years ago
|
||
Pushed by bjones@mozilla.com: https://hg.mozilla.org/hgcustom/version-control-tools/rev/199c089da890 MozReview: Remove rbbz/errors.py . r=smacleod https://hg.mozilla.org/hgcustom/version-control-tools/rev/71aa995b168a MozReview: Migrate rbbz/diffs.py to mozreview/diffs.py . r=smacleod https://hg.mozilla.org/hgcustom/version-control-tools/rev/57d3be17d927 MozReview: Migrate rbbz/forms.py to mozreview/forms.py . r=smacleod https://hg.mozilla.org/hgcustom/version-control-tools/rev/99f2b326360a MozReview: Migrate rbbz/middleware.py to mozreview/middleware.py . r=smacleod https://hg.mozilla.org/hgcustom/version-control-tools/rev/bd8d1a59a508 MozReview: Remove rbbz/resources (BugzillaCookieLoginResource) as it doesn't appear to be used . r=smacleod https://hg.mozilla.org/hgcustom/version-control-tools/rev/6a3a3391890f MozReview: Move rbbz/auth to mozreview extension . r=smacleod https://hg.mozilla.org/hgcustom/version-control-tools/rev/c1c14f237adb MozReview: Remove rbbz documentation . r=smacleod https://hg.mozilla.org/hgcustom/version-control-tools/rev/25f8ed25a92b MozReview: Move 'on_review_publishing' signal to mozreview . r=smacleod https://hg.mozilla.org/hgcustom/version-control-tools/rev/6cf0d893cebe MozReview: Move 'on_reply_publishing' signal to mozreview . r=smacleod https://hg.mozilla.org/hgcustom/version-control-tools/rev/661f9d930c98 MozReview: Remove rbbz extension and artifacts . r=smacleod
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 110•8 years ago
|
||
https://reviewboard.mozilla.org/r/55822/#review55682 > (This isn't necessarily a code issue you need to fix, just something I must explicitly call out). > > Deployment for this change could get a little interesting - what I believe might happen if we deploy as we normally do is the following: > > - We'll end up with all of the `rbbz` functionality inside of `MozReview`. > - The old `rbbz` package will still be kicking around, with all of the duplicated functionality. > - The old rbbz extension information will be kicking around in the database. > - The old rbbz extension will be enabled. > > I'm not exactly sure how all of this will interact, or if we can get away with just turning rbbz off / uninstalling it manually after the deploy. The best bet though would be to write a new playbook which will do this all automatically. Dropping because this has been pushed to production and completed by :glob.
You need to log in
before you can comment on or make changes to this bug.
Description
•