[damned-lies] Refactored cherry-pick stuff to allow displaying a proper success/failure message
- From: Claude Paroz <claudep src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [damned-lies] Refactored cherry-pick stuff to allow displaying a proper success/failure message
- Date: Sat, 17 Oct 2015 15:47:34 +0000 (UTC)
commit ce470aff0cbe0060ac2ca475004aba68ab22b56a
Author: Claude Paroz <claude 2xlibre net>
Date: Sat Oct 17 17:47:12 2015 +0200
Refactored cherry-pick stuff to allow displaying a proper success/failure message
stats/models.py | 13 ++++++++-----
stats/tests/tests.py | 16 +++++++---------
vertimus/forms.py | 2 +-
vertimus/models.py | 17 +++++++++++++----
vertimus/tests/tests.py | 7 ++++---
vertimus/views.py | 5 ++++-
6 files changed, 37 insertions(+), 23 deletions(-)
---
diff --git a/stats/models.py b/stats/models.py
index 93fdaa5..35c5b06 100644
--- a/stats/models.py
+++ b/stats/models.py
@@ -654,7 +654,7 @@ class Branch(models.Model):
else:
return command_list
- def commit_po(self, po_file, domain, language, author, sync_master=False):
+ def commit_po(self, po_file, domain, language, author):
""" Commit the file 'po_file' in the branch VCS repository """
if self.is_vcs_readonly():
raise Exception("This branch is in read-only mode. Unable to commit")
@@ -716,12 +716,13 @@ class Branch(models.Model):
stat.update_stats(dest_path)
else:
self.update_stats(force=False, checkout=False, domain=domain)
-
- if sync_master and not self.is_head():
- # Cherry-pick the commit on the master branch
- self.module.get_head_branch().cherrypick_commit(commit_hash, domain)
+ return commit_hash
def cherrypick_commit(self, commit_hash, domain):
+ """
+ Try to cherry-pick a branch commit `commit_hash` into master.
+ Return True if the cherry-pick succeeds, False otherwise.
+ """
if self.module.vcs_type != "git":
raise NotImplementedError("Commit cherry-pick is not implemented for '%s'" %
self.module.vcs_type)
with ModuleLock(self.module):
@@ -732,9 +733,11 @@ class Branch(models.Model):
if result[0] == utils.STATUS_OK:
utils.run_shell_command(
['git', 'push', 'origin', self.name], raise_on_error=True, cwd=commit_dir)
+ return True
else:
# Revert
utils.run_shell_command(['git', 'cherry-pick', '--abort'], cwd=commit_dir)
+ return False
DOMAIN_TYPE_CHOICES = (
diff --git a/stats/tests/tests.py b/stats/tests/tests.py
index fb45b46..0d0add2 100644
--- a/stats/tests/tests.py
+++ b/stats/tests/tests.py
@@ -312,7 +312,8 @@ class ModuleTestCase(TestCase):
'git add fr.po',
# Quoting is done at the Popen level
'git commit -m Updated French translation --author Author <someone example org>',
- 'git push origin master'
+ 'git push origin master', 'git log -n1 --format=oneline',
+ 'msgfmt --statistics -o /dev/null',
)
with patch_shell_command() as cmds:
branch.commit_po(po_file, domain, fr_lang, 'Author <someone example org>')
@@ -368,21 +369,18 @@ class ModuleTestCase(TestCase):
self.mod.vcs_root = self.mod.vcs_root.replace('git://', 'ssh://')
self.mod.save()
+ with patch_shell_command(only=['git push', 'git fetch', 'git reset']) as cmds:
+ commit_hash = branch.commit_po(po_file, domain, fr_lang, 'Author <someone example org>')
update_repo_sequence = (
- 'git checkout -f gnome-3-18', 'git fetch', 'git reset --hard origin/gnome-3-18',
+ 'git checkout -f master', 'git fetch', 'git reset --hard origin/master',
'git clean -dfq', 'if [ -e .gitmodules ]; then git submodule update --init; fi',
)
- update_repo_sequence_master = tuple(cmd.replace('gnome-3-18', 'master') for cmd in
update_repo_sequence)
git_ops = update_repo_sequence + (
- 'git add fr.po',
- 'git commit -m Updated French translation --author Author <someone example org>',
- 'git push origin gnome-3-18', 'git log -n1 --format=oneline', 'msgfmt --statistics -o /dev/null',
- ) + update_repo_sequence_master + (
'git cherry-pick -x',
'git push origin master',
)
- with patch_shell_command(only=['git pull', 'git push', 'git fetch', 'git reset']) as cmds:
- branch.commit_po(po_file, domain, fr_lang, 'Author <someone example org>', sync_master=True)
+ with patch_shell_command(only=['git push', 'git fetch', 'git reset']) as cmds:
+ self.branch.cherrypick_commit(commit_hash, domain)
for idx, cmd in enumerate(git_ops):
self.assertIn(cmd, cmds[idx])
diff --git a/vertimus/forms.py b/vertimus/forms.py
index f031573..70b36f3 100644
--- a/vertimus/forms.py
+++ b/vertimus/forms.py
@@ -91,7 +91,7 @@ class ActionForm(forms.Form):
self.fields['author'].initial = state.get_latest_po_file_action().person
if not has_mailing_list:
del self.fields['send_to_ml']
- if state.branch.is_head():
+ if state and state.branch.is_head():
del self.fields['sync_master']
def clean_file(self):
diff --git a/vertimus/models.py b/vertimus/models.py
index 924c5fb..840e667 100644
--- a/vertimus/models.py
+++ b/vertimus/models.py
@@ -638,16 +638,25 @@ class ActionCI(Action):
def apply_on(self, state, form_data):
self.state_db = state
action_with_po = self.get_previous_action_with_po()
+ author = form_data['author'].as_author() if form_data['author'] else None
try:
- author = form_data['author'].as_author() if form_data['author'] else None
- sync = form_data.get('sync_master', False)
state.branch.commit_po(
- action_with_po.file.path, state.domain, state.language, author, sync_master=sync)
+ action_with_po.file.path, state.domain, state.language, author)
except Exception:
# Commit failed, state unchanged
raise Exception(_("The commit failed. The error was: '%s'") % sys.exc_info()[1])
- super(ActionCI, self).apply_on(state, form_data) # Mail sent in super
+ msg = ugettext("The file has been successfully committed to the repository.")
+ if form_data.get('sync_master', False) and not state.branch.is_head():
+ # Cherry-pick the commit on the master branch
+ success = state.branch.module.get_head_branch().cherrypick_commit(commit_hash, state.domain)
+ if success:
+ msg += ugettext(" Additionally, the synchronization with the master branch succeeded.")
+ else:
+ msg += ugettext(" However, the synchronization with the master branch failed.")
+
+ super(ActionCI, self).apply_on(state, form_data) # Mail sent in super
+ return msg
class ActionRC(Action):
name = 'RC'
diff --git a/vertimus/tests/tests.py b/vertimus/tests/tests.py
index aab6e1f..80a00ad 100644
--- a/vertimus/tests/tests.py
+++ b/vertimus/tests/tests.py
@@ -391,9 +391,10 @@ class VertimusTest(TeamsAndRolesTests):
self.assertTrue(form.is_valid())
with patch_shell_command() as cmds:
action = Action.new_by_name('CI', person=self.pcoo)
- action.apply_on(state, form.cleaned_data)
- self.assertIn("git commit", cmds[-2])
- self.assertNotIn("--author", cmds[-2])
+ msg = action.apply_on(state, form.cleaned_data)
+ self.assertIn("git commit", cmds[-3])
+ self.assertNotIn("--author", cmds[-3])
+ self.assertEqual(msg, 'The file has been successfully committed to the repository.')
def test_action_ic(self):
diff --git a/vertimus/views.py b/vertimus/views.py
index dfb2463..bf2ac4e 100644
--- a/vertimus/views.py
+++ b/vertimus/views.py
@@ -119,13 +119,16 @@ def vertimus(request, branch, domain, language, stats=None, level="0"):
action = Action.new_by_name(action, person=person,
file=request.FILES.get('file', None))
try:
- action.apply_on(state, action_form.cleaned_data)
+ msg = action.apply_on(state, action_form.cleaned_data)
except SendMailFailed:
messages.error(request,
_("A problem occurred while sending mail, no mail have been sent"))
except Exception as e:
messages.error(request,
_("An error occurred during applying your action: %s") % e)
+ else:
+ if msg:
+ messages.success(request, msg)
return HttpResponseRedirect(
urlresolvers.reverse('vertimus_by_names',
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]