Details
- Reviewers
- None
- Group Reviewers
hg-reviewers
Diff Detail
- Lint
Lint Skipped - Unit
Unit Tests Skipped
Event Timeline
+#if updatetime option is turned on
+ $ hg amend --date '1997-1-1 0:1'
Use --config unless you need to re-run the whole tests with various options.
+ $ hg log --limit 2
+ # changeset: 40473:a092f9df5a43
+ # branch: stable
+ # bookmark: @
+ # tag: tip
+ # parent: 40406:7b48c616431d
+ # user: Taapas Agrawal<taapas2897@gmail.com>
+ # date: Wed Jan 01 00:01:00 1997 +0530
+ # summary: amend:Added current time update config option(issue5828)
Huh? What happened here?
+coreconfigitem('experimental', 'updatetime',
+ default=False,
+)
Martin showed a better name. rewrite.update-timestamp sounds good to me.
(I don't know which is a proper term, timestamp or time-stamp.)
https://phab.mercurial-scm.org/D4876#74017
+ update = ui.configbool('experimental','updatetime')
+ if update==True:
+ date=dateutil.parsedate(datetime.datetime.today().strftime("%Y-%m-%d %H:%M:%S"))
See commands.graft() for how to get a current timestamp.
In the tests I added two parts one with config option False and and another with True. I will correct the heading that part is with config turned off.
Okay I will change the config option to
rewrite.updatetimestamp
and will see how to get current timestamp from commands.graft() .
+$ hg amend --config rewrite.updatetimestamp=True
+$ hg log --limit 2
+changeset: 40473:e64479cbaf37
Okay, so these tests don't run because these lines don't start with $.
+coreconfigitem('rewrite', 'updatetimestamp',
Need dash per https://www.mercurial-scm.org/wiki/UIGuideline#config
And please update mercurial/help/config.txt as well.
diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -2545,6 +2545,10 @@pureextra = extra.copy() extra['amend_source'] = old.hex()+ updatetime = ui.configbool('rewrite','updatetimestamp')
+ if updatetime==True:
if updatetime: (or simply if ui.configbool(...):)
@yuja Corrected the tests. Added in config.txt and changed the config option according to the guideline. Please review when you are free.
Hi, thanks for the patch.
- After this patch, amend will set the date to current date even if --date is passed, which is not preferred. The correct order should be, --date if passed, else the current date if new config is set, else old date. Also the current date should only be set if there are more changes to amend.
- If I understand correctly, you are manually updating the tests. You should have a look at https://www.mercurial-scm.org/wiki/WritingTests#Running_the_test_suite.
- Please add more details in the commit message about the behavior of the new config option.
@pulkit I have updated the revision. Sorry about the tests.Please review when you are free.
The patch is going in a good direction. We now need to make sure that we don't amend if date is the only thing which has changed.
tests/test-amend.t | ||
---|---|---|
382 | This one should not amend as there are no other changes than the date change. |
Nice! I left an inline comment.
How are you sending patches? I am unable to see the whole file in phabricator. If you are not using hg phabsend, please use that next time.
For how to use hg phabsend: https://www.mercurial-scm.org/wiki/Phabricator
mercurial/cmdutil.py | ||
---|---|---|
2559 | How about if we leave this if statement as it is and do following after the if statement:
nit: Return value of ui.configbool() is a boolean and you don't need to compare that tp True. |
I think that would not be correct because we need to avoid date change update-timestamp is true and when no option is passed and no attribute is changed . Only when there is change in any of the attributes then only date is to be updated which can only be tested this way only.
oh i was using update diff button to update the patch . I will resend a new patch using
hg phabsend
How about if we leave this if statement as it is and do following after the if statement:
nit: Return value of ui.configbool() is a boolean and you don't need to compare that tp True.