Issue 29336739: Add recommended hgrc (Closed)
|
Created:
Feb. 20, 2016, 11:59 a.m. by Felix Dahlke Modified:
Feb. 23, 2016, 4:31 p.m. Reviewers:
Wladimir Palant Visibility:
Public. |
DescriptionAdd recommended hgrc
Patch Set 1 #
Total comments: 19
Patch Set 2 : Address comments, plus some other changes #Patch Set 3 : Use the same example user name as Mercurial #
Total comments: 16
Patch Set 4 : Address comments #MessagesTotal messages: 7
Here is my first take on this. Instead of just throwing a complicted config file at people, I want to add reasoning and explanations. I believe reading through this can help get people up to speed with Mercurial faster. (If not I probably just did a bad job.) What I still want to do is look through some nicely commented config files and see how they structure the comments. I guess there's some room for improvement below. https://codereview.adblockplus.org/29336739/diff/29336740/hgrc File hgrc (right): https://codereview.adblockplus.org/29336739/diff/29336740/hgrc#newcode68 hgrc:68: [alias] Any other suggestions for useful aliases I couldn't think of? Note that I haven't added the `update` -> `update -r master` alias you suggested because I still want to investigate if there really is no difference between `hg update` and `hg update -r default` - I think there is.
https://codereview.adblockplus.org/29336739/diff/29336740/hgrc File hgrc (right): https://codereview.adblockplus.org/29336739/diff/29336740/hgrc#newcode1 hgrc:1: # This .hgrc is recommended for working on the Adblock Plus project. General notes: 1) The order is wrong, obvious stuff ([ui], [defaults]) should go first, extensions and extension settings should be listed after that. 2) An essential setting is missing: [diff] git = true Without this Mercurial won't produce binary diffs. https://codereview.adblockplus.org/29336739/diff/29336740/hgrc#newcode20 hgrc:20: pager = I don't use that, find it pretty annoying actually and would love to disable in Git - my terminal has scrollback. I don't think such extensions which are really a matter of taste belong here, maybe as recommendation for something to look at. Nit: "shows in a pager" doesn't actually explain anything. Maybe "shows the output page by page"? https://codereview.adblockplus.org/29336739/diff/29336740/hgrc#newcode23 hgrc:23: color = Same here. While this is usually useful, it produces a horrible mess on Windows. Nits: colours => colors (we generally use AE), lke => like https://codereview.adblockplus.org/29336739/diff/29336740/hgrc#newcode26 hgrc:26: # longer. No comma after take. https://codereview.adblockplus.org/29336739/diff/29336740/hgrc#newcode38 hgrc:38: # allows you to clean unversioned files. Nit: You need to close that parenthesis somewhere. https://codereview.adblockplus.org/29336739/diff/29336740/hgrc#newcode44 hgrc:44: shelve = I couldn't find any use for that when working with bookmarked branches. https://codereview.adblockplus.org/29336739/diff/29336740/hgrc#newcode63 hgrc:63: #username = Jane Bloggs <jane@adblockplus.org> I hope we never hire a Jane... The explanation is rather strange. I think the second sentence should rather be: "You have to set this or all your commits will show up with a weird auto-generated author name." https://codereview.adblockplus.org/29336739/diff/29336740/hgrc#newcode85 hgrc:85: #push = push --rev master You are misusing aliases here, you should use [defaults] section instead: [defaults] log = --graph clone = --updaterev master push = --rev master outgoing = --rev master Note that I added outgoing, I forgot this one before.
New version up. https://codereview.adblockplus.org/29336739/diff/29336740/hgrc File hgrc (right): https://codereview.adblockplus.org/29336739/diff/29336740/hgrc#newcode1 hgrc:1: # This .hgrc is recommended for working on the Adblock Plus project. On 2016/02/20 17:50:48, Wladimir Palant wrote: > 1) The order is wrong, obvious stuff ([ui], [defaults]) should go first, > extensions and extension settings should be listed after that. Don't feel nearly as strong about that, changed it. > 2) An essential setting is missing: > > [diff] > git = true Added. Not sure how I got by without that so far. https://codereview.adblockplus.org/29336739/diff/29336740/hgrc#newcode20 hgrc:20: pager = On 2016/02/20 17:50:48, Wladimir Palant wrote: > I don't use that, find it pretty annoying actually and would love to disable in > Git - my terminal has scrollback. I don't think such extensions which are really > a matter of taste belong here, maybe as recommendation for something to look at. Hm, didn't think anyone would not want that. So you always specify `-l` I suppose? Made it opt-in then. > Nit: "shows in a pager" doesn't actually explain anything. Maybe "shows the > output page by page"? I think it does explain this adequately, a terminal pager is a thing. I've clarified this a bit more. https://codereview.adblockplus.org/29336739/diff/29336740/hgrc#newcode23 hgrc:23: color = On 2016/02/20 17:50:48, Wladimir Palant wrote: > Same here. While this is usually useful, it produces a horrible mess on Windows. > > Nits: colours => colors (we generally use AE), lke => like Done. https://codereview.adblockplus.org/29336739/diff/29336740/hgrc#newcode26 hgrc:26: # longer. On 2016/02/20 17:50:48, Wladimir Palant wrote: > No comma after take. Done. https://codereview.adblockplus.org/29336739/diff/29336740/hgrc#newcode38 hgrc:38: # allows you to clean unversioned files. On 2016/02/20 17:50:48, Wladimir Palant wrote: > Nit: You need to close that parenthesis somewhere. Done. https://codereview.adblockplus.org/29336739/diff/29336740/hgrc#newcode44 hgrc:44: shelve = On 2016/02/20 17:50:48, Wladimir Palant wrote: > I couldn't find any use for that when working with bookmarked branches. Not? I use it on a weekly basis, just like git stash. I don't always want to make a new branch out of my WIP changes when just quickly fixing a typo or something. Plus, it's more beginner friendly. No particularly strong opinion about not enabling it by default, but I don't think one extra command hurts. Not everyone will use histedit or record, either. https://codereview.adblockplus.org/29336739/diff/29336740/hgrc#newcode63 hgrc:63: #username = Jane Bloggs <jane@adblockplus.org> On 2016/02/20 17:50:48, Wladimir Palant wrote: > I hope we never hire a Jane... > > The explanation is rather strange. I think the second sentence should rather be: > "You have to set this or all your commits will show up with a weird > auto-generated author name." Is that what happens for you? I always get this: $ hg init foo $ cd foo $ touch bar $ hg add bar $ hg commit -m "Initial commit" abort: no username supplied (use "hg config --edit" to set your username) (Yes, I do not have username in ~/.hgrc, I'm working on non-ABP Mercurial projects.) https://codereview.adblockplus.org/29336739/diff/29336740/hgrc#newcode85 hgrc:85: #push = push --rev master On 2016/02/20 17:50:47, Wladimir Palant wrote: > You are misusing aliases here, you should use [defaults] section instead: > > [defaults] > log = --graph > clone = --updaterev master > push = --rev master > outgoing = --rev master > > Note that I added outgoing, I forgot this one before. Oh nice, forgot that that's possible. Using aliases for this seemed hacky. (Yes, I do not have any defaults or aliases currently.) Done.
https://codereview.adblockplus.org/29336739/diff/29336740/hgrc File hgrc (right): https://codereview.adblockplus.org/29336739/diff/29336740/hgrc#newcode20 hgrc:20: pager = On 2016/02/20 20:16:38, Felix Dahlke wrote: > Hm, didn't think anyone would not want that. So you always specify `-l` I > suppose? Made it opt-in then. Yes, I have "log = -l 5" set under [defaults]. https://codereview.adblockplus.org/29336739/diff/29336740/hgrc#newcode63 hgrc:63: #username = Jane Bloggs <jane@adblockplus.org> On 2016/02/20 20:16:38, Felix Dahlke wrote: > Is that what happens for you? I always get this: > abort: no username supplied > (use "hg config --edit" to set your username) Weird, same here - supposedly working like that since at least 2007 (https://www.selenic.com/hg/rev/78a0dd93db0b). I could verify that on Windows and OS X. Yet I'm certain that not too long ago someone again managed to push commits with a misconfigured user name... And I've seen that issue myself, yet I started using Mercurial after 2007. https://codereview.adblockplus.org/29336739/diff/29336785/hgrc File hgrc (right): https://codereview.adblockplus.org/29336739/diff/29336785/hgrc#newcode7 hgrc:7: # The commented parts do either not work on all systems, are a matter of taste, do either => either do https://codereview.adblockplus.org/29336739/diff/29336785/hgrc#newcode21 hgrc:21: # We ue Git's extended diff format which includes more information (such as ue => use https://codereview.adblockplus.org/29336739/diff/29336785/hgrc#newcode23 hgrc:23: git = true Might want to add the following: # Uncomment this if you want to see more than three lines of context on your # patches. Eight lines generally allow a better overview. #unified = 8 https://codereview.adblockplus.org/29336739/diff/29336785/hgrc#newcode32 hgrc:32: # the history a lot easier to read. How about adding: It will also mark the revision you are currently at. https://codereview.adblockplus.org/29336739/diff/29336785/hgrc#newcode45 hgrc:45: # any branch. You most likely want to update to the "master" bookmark instead, This isn't correct - hg clone updates to the latest commit on the default branch, it doesn't update to other named branches. You might want to add that specifying "hg clone --updaterev default" explicitly will restore the default behavior regardless of this setting. https://codereview.adblockplus.org/29336739/diff/29336785/hgrc#newcode52 hgrc:52: # you need to add `--rev master`. By uncommenting the following two lines, you Here as well, you can say that one can always specify --rev my-bookmark explicitly to override this default and push a different branch. https://codereview.adblockplus.org/29336739/diff/29336785/hgrc#newcode56 hgrc:56: So, what about update? I checked and there are two differences between "hg update" and "hg update -r default". First, the former will only update on a linear history. If you are on a different anonymous branch than the head of the default branch it will abort. This will save you in many cases. The second difference is the really critical one - hg update without a revision will automatically move bookmarks. Consider the following history: o * Bar [issue-12345] | @ * Foo [master] | o * Initial commit You've been working on the feature branch issue-12345, later you did "hg update -r master" to go back to the master bookmark. If you use "hg update" now and forget to specify an explicit revision you won't merely be moved to your feature branch, it will also move the master bookmark there! It's not a theoretical scenario but rather exactly the situation I have in adblockpluscore with the emscripten branch right now (and I actually had to fix the master bookmark already). IMHO that logic is quite a footgun, I don't really understand what they were thinking with the implicit moving of bookmarks on update. "update = --rev master" here will take care of that. And "hg update --rev default" will give you the default logic again - albeit without the bookmark moving behavior (a good thing IMHO) and crossing branches (fine as there are no side-effects). So I'd be very much in favor of adding this as a recommendation here.
New patch set is up. https://codereview.adblockplus.org/29336739/diff/29336785/hgrc File hgrc (right): https://codereview.adblockplus.org/29336739/diff/29336785/hgrc#newcode7 hgrc:7: # The commented parts do either not work on all systems, are a matter of taste, On 2016/02/22 11:05:27, Wladimir Palant wrote: > do either => either do Done. https://codereview.adblockplus.org/29336739/diff/29336785/hgrc#newcode21 hgrc:21: # We ue Git's extended diff format which includes more information (such as On 2016/02/22 11:05:27, Wladimir Palant wrote: > ue => use Done. https://codereview.adblockplus.org/29336739/diff/29336785/hgrc#newcode23 hgrc:23: git = true On 2016/02/22 11:05:27, Wladimir Palant wrote: > Might want to add the following: > > # Uncomment this if you want to see more than three lines of context on your > # patches. Eight lines generally allow a better overview. > #unified = 8 Done. Made it a default since I believe it's saner than three lines of context. https://codereview.adblockplus.org/29336739/diff/29336785/hgrc#newcode32 hgrc:32: # the history a lot easier to read. On 2016/02/22 11:05:28, Wladimir Palant wrote: > How about adding: It will also mark the revision you are currently at. Done. https://codereview.adblockplus.org/29336739/diff/29336785/hgrc#newcode45 hgrc:45: # any branch. You most likely want to update to the "master" bookmark instead, On 2016/02/22 11:05:27, Wladimir Palant wrote: > This isn't correct - hg clone updates to the latest commit on the default > branch, it doesn't update to other named branches. Oh indeed, I was pretty sure it would update to tip... > You might want to add that specifying "hg clone --updaterev default" explicitly > will restore the default behavior regardless of this setting. Done. https://codereview.adblockplus.org/29336739/diff/29336785/hgrc#newcode52 hgrc:52: # you need to add `--rev master`. By uncommenting the following two lines, you On 2016/02/22 11:05:27, Wladimir Palant wrote: > Here as well, you can say that one can always specify --rev my-bookmark > explicitly to override this default and push a different branch. Done. https://codereview.adblockplus.org/29336739/diff/29336785/hgrc#newcode56 hgrc:56: On 2016/02/22 11:05:27, Wladimir Palant wrote: > So, what about update? I checked and there are two differences between "hg > update" and "hg update -r default". First, the former will only update on a > linear history. If you are on a different anonymous branch than the head of the > default branch it will abort. This will save you in many cases. > > The second difference is the really critical one - hg update without a revision > will automatically move bookmarks. Consider the following history: > > o * Bar [issue-12345] > | > @ * Foo [master] > | > o * Initial commit > > You've been working on the feature branch issue-12345, later you did "hg update > -r master" to go back to the master bookmark. If you use "hg update" now and > forget to specify an explicit revision you won't merely be moved to your feature > branch, it will also move the master bookmark there! It's not a theoretical > scenario but rather exactly the situation I have in adblockpluscore with the > emscripten branch right now (and I actually had to fix the master bookmark > already). > > IMHO that logic is quite a footgun, I don't really understand what they were > thinking with the implicit moving of bookmarks on update. "update = --rev > master" here will take care of that. And "hg update --rev default" will give you > the default logic again - albeit without the bookmark moving behavior (a good > thing IMHO) and crossing branches (fine as there are no side-effects). > > So I'd be very much in favor of adding this as a recommendation here. Yes me too, we're both pretty used to Mercurial by now and we had no idea how `hg update` works. I had this all added. For the record, I did it like this: ---------- # `hg update`, when used without arguments, has somewhat questionable behaviour. # It will update the working directory to the head of the current branch, and # move the active bookmark while at it. We believe it is less error prone if # running `hg update` without rev updates to the main branch instead. update = --rev default # With the above, you will still need to specify `--rev master` when working # with repositories that have a "master" branch. If you uncomment the following # instead, you won't have to worry about that. #update = --rev master ---------- BUT: ---------- $ hg update feature abort: please specify just one revision ---------- As we both know, --rev is optional. Most documentation I've seen doesn't specify the option, nor do I use it in practice. Seems that when --rev is not specified, Mercurial doesn't overwrite the default and adds it redundantly instead... That makes me not want to add this default. The error message is too cryptic and it's too inconvenient.
LGTM https://codereview.adblockplus.org/29336739/diff/29336785/hgrc File hgrc (right): https://codereview.adblockplus.org/29336739/diff/29336785/hgrc#newcode56 hgrc:56: On 2016/02/22 22:16:53, Felix Dahlke wrote: > That makes me not want to add this default. The error message is too cryptic and > it's too inconvenient. Ouch... As you probably noticed, I always specify the -r parameter - I don't want to remember that for hg update it is optional but for hg revert it is not. But well, then we just have to make sure that we explain this behavior somewhere.
Thanks, landed: https://hg.adblockplus.org/codingtools/rev/f531003d1ae5 https://codereview.adblockplus.org/29336739/diff/29336785/hgrc File hgrc (right): https://codereview.adblockplus.org/29336739/diff/29336785/hgrc#newcode56 hgrc:56: On 2016/02/23 10:52:18, Wladimir Palant wrote: > On 2016/02/22 22:16:53, Felix Dahlke wrote: > > That makes me not want to add this default. The error message is too cryptic > and > > it's too inconvenient. > > Ouch... As you probably noticed, I always specify the -r parameter - I don't > want to remember that for hg update it is optional but for hg revert it is not. > But well, then we just have to make sure that we explain this behavior > somewhere. Yeah, my vote is for getting started. Wanna add that? 
|
