Merge lp:~asac/bzr/lp459276 into lp:bzr

Proposed by Alexander Sack
Status: Merged
Approved by: Andrew Bennetts
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~asac/bzr/lp459276
Merge into: lp:bzr
Diff against target: 63 lines
1 file modified
bzrlib/builtins.py (+13/-5)
To merge this branch: bzr merge lp:~asac/bzr/lp459276
Reviewer Review Type Date Requested Status
Andrew Bennetts Approve
Ian Clatworthy Approve
Review via email: mp+13861@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Alexander Sack (asac) wrote :

add --commit-time option to the commit built-in command ... requested in lp:459276

Revision history for this message
Ian Clatworthy (ian-clatworthy) wrote :

Alexander,

Thanks for the patch. I'm ok with the concept though I wonder if some other core members might have concerns? It might be worth a short email to the list explaining the problem and outlining your suggested improvement. Others can then provide input as to whether we ought to solve it another way or in the way you've proposed.

The code itself looks fine. Assuming there's agreement on doing this, it really needs some tests though before we can land it. Some other minor things:

* Please add a '.' to the end of the commit-time help to pass our pre-commit checks.
* Please add an entry to NEWS.

review: Needs Fixing
Revision history for this message
Robert Collins (lifeless) wrote :

No concerns here, we don't trust timestamps anyway.

-Rob

Revision history for this message
Ian Clatworthy (ian-clatworthy) wrote :

OK, this is now agreed to conceptually. The next step under our new process is for Andrew to "pilot" this until it lands.

review: Approve
Revision history for this message
Andrew Bennetts (spiv) wrote :
Download full text (3.7 KiB)

This looks good to me. I've implemented Ian's review suggestions (added tests, added NEWS entry, and added '.' to the end of the option help). I also fixed it to preserve the timezone specified in the --commit-time.

I'm submitting it to PQM now.

Here's the relevant diff of my changes:

=== modified file 'NEWS'
--- NEWS 2009-11-18 22:03:00 +0000
+++ NEWS 2009-11-19 06:30:25 +0000
@@ -23,6 +23,9 @@
 New Features
 ************

+* ``bzr commit`` now has a ``--commit-time`` option.
+ (Alexander Sack, #459276)
+
 Bug Fixes
 *********

=== modified file 'bzrlib/builtins.py'
--- bzrlib/builtins.py 2009-10-23 18:00:44 +0000
+++ bzrlib/builtins.py 2009-11-19 06:27:16 +0000
@@ -2943,8 +2943,8 @@
                     help="Refuse to commit if there are unknown "
                     "files in the working tree."),
              Option('commit-time', type=str,
- help="Manually set a commit time using commit date format, e.g."
- "'2009-10-10 08:00:00 +0100'"),
+ help="Manually set a commit time using commit date "
+ "format, e.g. '2009-10-10 08:00:00 +0100'."),
              ListOption('fixes', type=str,
                     help="Mark a bug as being fixed by this revision "
                          "(see \"bzr help bugs\")."),
@@ -2996,9 +2996,13 @@
             make_commit_message_template_encoded
         )

- commit_stamp = None
+ commit_stamp = offset = None
         if commit_time is not None:
+ try:
                 commit_stamp, offset = timestamp.parse_patch_date(commit_time)
+ except ValueError, e:
+ raise errors.BzrCommandError(
+ "Could not parse --commit-time: " + str(e))

         # TODO: Need a blackbox test for invoking the external editor; may be
         # slightly problematic to run this cross-platform.
@@ -3058,6 +3062,7 @@
                         allow_pointless=unchanged, strict=strict, local=local,
                         reporter=None, verbose=verbose, revprops=properties,
                         authors=author, timestamp=commit_stamp,
+ timezone=offset,
                         exclude=safe_relpath_files(tree, exclude))
         except PointlessCommit:
             # FIXME: This should really happen before the file is read in;

=== modified file 'bzrlib/tests/blackbox/test_commit.py'
--- bzrlib/tests/blackbox/test_commit.py 2009-08-28 05:00:33 +0000
+++ bzrlib/tests/blackbox/test_commit.py 2009-11-19 06:27:16 +0000
@@ -590,6 +590,26 @@
         properties = last_rev.properties
         self.assertEqual('John Doe\nJane Rey', properties['authors'])

+ def test_commit_time(self):
+ tree = self.make_branch_and_tree('tree')
+ self.build_tree(['tree/hello.txt'])
+ tree.add('hello.txt')
+ out, err = self.run_bzr("commit -m hello "
+ "--commit-time='2009-10-10 08:00:00 +0100' tree/hello.txt")
+ last_rev = tree.branch.repository.get_revision(tree.last_revision())
+ self.assertEqual(
+ 'Sat 2009-10-10 08:00:00 +0100',
+ osutils.format_date(last_rev.timestamp, last_rev.timezone))
+ ...

Read more...

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bzrlib/builtins.py'
2--- bzrlib/builtins.py 2009-10-12 22:09:19 +0000
3+++ bzrlib/builtins.py 2009-10-23 18:05:24 +0000
4@@ -44,6 +44,7 @@
5 rename_map,
6 revision as _mod_revision,
7 symbol_versioning,
8+ timestamp,
9 transport,
10 ui,
11 urlutils,
12@@ -2941,6 +2942,9 @@
13 Option('strict',
14 help="Refuse to commit if there are unknown "
15 "files in the working tree."),
16+ Option('commit-time', type=str,
17+ help="Manually set a commit time using commit date format, e.g."
18+ "'2009-10-10 08:00:00 +0100'"),
19 ListOption('fixes', type=str,
20 help="Mark a bug as being fixed by this revision "
21 "(see \"bzr help bugs\")."),
22@@ -2953,9 +2957,9 @@
23 "the master branch until a normal commit "
24 "is performed."
25 ),
26- Option('show-diff',
27- help='When no message is supplied, show the diff along'
28- ' with the status summary in the message editor.'),
29+ Option('show-diff',
30+ help='When no message is supplied, show the diff along'
31+ ' with the status summary in the message editor.'),
32 ]
33 aliases = ['ci', 'checkin']
34
35@@ -2980,7 +2984,7 @@
36
37 def run(self, message=None, file=None, verbose=False, selected_list=None,
38 unchanged=False, strict=False, local=False, fixes=None,
39- author=None, show_diff=False, exclude=None):
40+ author=None, show_diff=False, exclude=None, commit_time=None):
41 from bzrlib.errors import (
42 PointlessCommit,
43 ConflictsInTree,
44@@ -2992,6 +2996,10 @@
45 make_commit_message_template_encoded
46 )
47
48+ commit_stamp = None
49+ if commit_time is not None:
50+ commit_stamp, offset = timestamp.parse_patch_date(commit_time)
51+
52 # TODO: Need a blackbox test for invoking the external editor; may be
53 # slightly problematic to run this cross-platform.
54
55@@ -3049,7 +3057,7 @@
56 specific_files=selected_list,
57 allow_pointless=unchanged, strict=strict, local=local,
58 reporter=None, verbose=verbose, revprops=properties,
59- authors=author,
60+ authors=author, timestamp=commit_stamp,
61 exclude=safe_relpath_files(tree, exclude))
62 except PointlessCommit:
63 # FIXME: This should really happen before the file is read in;