Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(510)

Issue 131540043: Do not treat auto_generated messages as published messages (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years ago by rmistry
Modified:
11 years ago
Visibility:
Public.

Description

Do not treat auto_generated messages as published messages. This is to ensure that no matter what logging is done before (reviewers added, CQ checked, etc), the first published message contains the diff and does not contain 'Re:'. As part of this CL I also reverted the workarounds done in: * https://codereview.appspot.com/126680043/ (Delay the creation of the reviewers change log message to avoid "Re:" subject on initial comment) * https://codereview.appspot.com/133880043/ (Fix initial diff email feature by calling put() on auto-generated Messages later) This CL replaces <a href='https://code.google.com/p/rietveld/source/detail?r=af712bf0708b4ec1043c2783e197a75959a40006&name=chromium'>https://code.google.com/p/rietveld/source/detail?r=af712bf0708b4ec1043c2783e197a75959a40006&name=chromium</a> with a better approach. Tested with the following CLs: * https://skia-codereview-staging.appspot.com/6841001/ (testing add-reviewers-in-edit-then-publish) * https://skia-codereview-staging.appspot.com/1871001/ (testing add-reviewers-in-publish) * https://skia-codereview-staging.appspot.com/8831001/ (testing check-cq-then-publish) BUG=<a href='https://code.google.com/p/chromium/issues/detail?id=407382'>chromium:407382</a>

Patch Set 1 : Initial upload #

Patch Set 2 : Cleanup #

Patch Set 3 : Making sure nothing changed locally #

Unified diffs Side-by-side diffs Delta from patch set Stats (+25 lines, -20 lines) Patch
M codereview/models.py View 1 1 chunk +5 lines, -1 line 0 comments Download
M codereview/views.py View 1 7 chunks +13 lines, -19 lines 0 comments Download
M index.yaml View 1 chunk +7 lines, -0 lines 0 comments Download

Messages

Total messages: 5
rmistry
11 years ago (2014-08-28 15:25:52 UTC) #1
jrobbins (corp)
lgtm You might want to note in the CL description that this replaces https://code.google.com/p/rietveld/source/detail?r=af712bf0708b4ec1043c2783e197a75959a40006&name=chromium with ...
11 years ago (2014-08-28 20:43:36 UTC) #2
rmistry
On 2014/08/28 20:43:36, jrobbins (corp) wrote: > lgtm > > You might want to note ...
11 years ago (2014-08-28 20:45:29 UTC) #3
rmistry
Submitted as https://code.google.com/p/rietveld/source/detail?r=1eebc533dbbcdb65ec222b07ca295b263bd4d604&name=chromium
11 years ago (2014-08-28 20:47:51 UTC) #4
rmistry
11 years ago (2014-08-28 20:49:02 UTC) #5
Message was sent while issue was closed.
Whenever this is deployed (to a non-default version), lets try to test all edit
+ publish flows we can.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b