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

Issue 29508667: Issue 4354, 4355 - handle dirty/outdated repos on release (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 6 months ago by tlucas
Modified:
2 years, 6 months ago
Visibility:
Public.

Description

Issues 4354, 4355 - handle dirty / outdated (mercurial)-repositories in releaseAutomation

Patch Set 1 #

Total comments: 2

Patch Set 2 : #

Total comments: 28

Patch Set 3 : Adjusting error message, removing complexity, increasing readability #

Total comments: 8

Patch Set 4 : Raise errors on unexpected return-codes #

Patch Set 5 : Move sanity-function to toplevel #

Total comments: 4

Patch Set 6 : #

Total comments: 15

Patch Set 7 : removed obsolete print #

Patch Set 8 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+71 lines, -0 lines) Patch
M releaseAutomation.py View 1 2 3 4 5 6 7 2 chunks +71 lines, -0 lines 0 comments Download

Messages

Total messages: 25
tlucas
Patch Set 1 Implemented checks of whether repositories are dirty / outdated https://issues.adblockplus.org/ticket/4354 https://issues.adblockplus.org/ticket/4355
2 years, 6 months ago (2017-08-07 14:19:38 UTC) #1
kzar
https://codereview.adblockplus.org/29508667/diff/29508668/build.py File build.py (right): https://codereview.adblockplus.org/29508667/diff/29508668/build.py#newcode427 build.py:427: path = 'default' This change seems unrelated?
2 years, 6 months ago (2017-08-07 14:59:59 UTC) #2
tlucas
Patch Set 2 Removed unrelated changes (response to kzar's comment) https://codereview.adblockplus.org/29508667/diff/29508668/build.py File build.py (right): https://codereview.adblockplus.org/29508667/diff/29508668/build.py#newcode427 ...
2 years, 6 months ago (2017-08-07 15:42:07 UTC) #3
Vasily Kuznetsov
Hi Tristan, The general outline looks allright, but I have some comments about readability and ...
2 years, 6 months ago (2017-08-08 18:10:12 UTC) #4
tlucas
Hey guys and thank you Vasily, for the review! I'll comment on your comments and ...
2 years, 6 months ago (2017-08-09 08:12:36 UTC) #5
Vasily Kuznetsov
https://codereview.adblockplus.org/29508667/diff/29508684/releaseAutomation.py File releaseAutomation.py (right): https://codereview.adblockplus.org/29508667/diff/29508684/releaseAutomation.py#newcode106 releaseAutomation.py:106: input = raw_input On 2017/08/09 08:12:34, tlucas wrote: > ...
2 years, 6 months ago (2017-08-09 17:09:10 UTC) #6
tlucas
Patch Set 3 * Changed to the new agreed error-message * Do not print local ...
2 years, 6 months ago (2017-08-15 14:40:13 UTC) #7
Sebastian Noack
https://codereview.adblockplus.org/29508667/diff/29516572/releaseAutomation.py File releaseAutomation.py (right): https://codereview.adblockplus.org/29508667/diff/29516572/releaseAutomation.py#newcode4 releaseAutomation.py:4: from __future__ import print_function I would add a blank ...
2 years, 6 months ago (2017-08-15 15:54:46 UTC) #8
tlucas
Patch Set 4 * Raising Exception on unexpected return-codes from hg-calls * Commented on the ...
2 years, 6 months ago (2017-08-16 08:38:16 UTC) #9
Sebastian Noack
https://codereview.adblockplus.org/29508667/diff/29516572/releaseAutomation.py File releaseAutomation.py (right): https://codereview.adblockplus.org/29508667/diff/29516572/releaseAutomation.py#newcode106 releaseAutomation.py:106: def can_safely_release(): On 2017/08/16 08:38:15, tlucas wrote: > On ...
2 years, 6 months ago (2017-08-16 08:48:36 UTC) #10
tlucas
Patch Set 5 Moved sanity-check-function to top-level https://codereview.adblockplus.org/29508667/diff/29516572/releaseAutomation.py File releaseAutomation.py (right): https://codereview.adblockplus.org/29508667/diff/29516572/releaseAutomation.py#newcode106 releaseAutomation.py:106: def can_safely_release(): ...
2 years, 6 months ago (2017-08-16 08:53:24 UTC) #11
Sebastian Noack
https://codereview.adblockplus.org/29508667/diff/29517557/releaseAutomation.py File releaseAutomation.py (right): https://codereview.adblockplus.org/29508667/diff/29517557/releaseAutomation.py#newcode4 releaseAutomation.py:4: from __future__ import print_function There should be a blank ...
2 years, 6 months ago (2017-08-16 09:16:14 UTC) #12
tlucas
Patch Set 6 Addressed Sebastian's comments on Patch Set 5 (newline / docstring) https://codereview.adblockplus.org/29508667/diff/29517557/releaseAutomation.py File ...
2 years, 6 months ago (2017-08-16 10:24:25 UTC) #13
Sebastian Noack
https://codereview.adblockplus.org/29508667/diff/29517565/releaseAutomation.py File releaseAutomation.py (right): https://codereview.adblockplus.org/29508667/diff/29517565/releaseAutomation.py#newcode104 releaseAutomation.py:104: print('Please answer "yes" or "no"!') This message seems redundant. ...
2 years, 6 months ago (2017-08-16 10:33:29 UTC) #14
tlucas
Patch Set 7 Remove obsolete print https://codereview.adblockplus.org/29508667/diff/29517565/releaseAutomation.py File releaseAutomation.py (right): https://codereview.adblockplus.org/29508667/diff/29517565/releaseAutomation.py#newcode104 releaseAutomation.py:104: print('Please answer "yes" ...
2 years, 6 months ago (2017-08-16 10:40:30 UTC) #15
Sebastian Noack
LGTM
2 years, 6 months ago (2017-08-16 10:46:07 UTC) #16
Wladimir Palant
https://codereview.adblockplus.org/29508667/diff/29517565/releaseAutomation.py File releaseAutomation.py (right): https://codereview.adblockplus.org/29508667/diff/29517565/releaseAutomation.py#newcode81 releaseAutomation.py:81: incoming = True This seems inconsistent compared to repo_has_outgoing() ...
2 years, 6 months ago (2017-08-16 10:46:09 UTC) #17
tlucas
Hey Wladimir, thank you for the review! I think we should discuss a bit more ...
2 years, 6 months ago (2017-08-16 11:05:19 UTC) #18
Wladimir Palant
https://codereview.adblockplus.org/29508667/diff/29517565/releaseAutomation.py File releaseAutomation.py (right): https://codereview.adblockplus.org/29508667/diff/29517565/releaseAutomation.py#newcode81 releaseAutomation.py:81: incoming = True On 2017/08/16 11:05:19, tlucas wrote: > ...
2 years, 6 months ago (2017-08-16 11:11:46 UTC) #19
tlucas
Patch Set 8 * Re-added lost parameters to callers / callees * addressed comments from ...
2 years, 6 months ago (2017-08-16 11:27:55 UTC) #20
Wladimir Palant
LGTM Now is there any easy way to also make errors in the ensure_dependencies.py call ...
2 years, 6 months ago (2017-08-16 11:47:22 UTC) #21
Sebastian Noack
LGTM
2 years, 6 months ago (2017-08-16 11:48:57 UTC) #22
tlucas
On 2017/08/16 11:47:22, Wladimir Palant wrote: > LGTM > > Now is there any easy ...
2 years, 6 months ago (2017-08-16 12:04:49 UTC) #23
kzar
On 2017/08/16 12:04:49, tlucas wrote: > On 2017/08/16 11:47:22, Wladimir Palant wrote: > > LGTM ...
2 years, 6 months ago (2017-08-17 13:12:10 UTC) #24
Wladimir Palant
2 years, 6 months ago (2017-08-17 13:21:38 UTC) #25
Message was sent while issue was closed.
On 2017/08/17 13:12:10, kzar wrote:
> We should probably open a separate issue for that though.

Definitely. This was merely meant as an additional idea to improve build
consistency, not really critical however - only worth doing if easy.
Sign in to reply to this message.

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