< bitcoin-git>
[bitcoin] achow101 opened pull request #10952: [wallet] Remove vchDefaultKey and have better first run detection (master...remove-defaultkey) https://github.com/bitcoin/bitcoin/pull/10952
< sam_c>
achow101: FYI wrt pr 10952, you can use git add -p to only add parts of the local changes
< sam_c>
achow101: this prevents whitespace or other unintentional changes from finding their way into commit.
< sam_c>
bit late for that one but good to know :)
< jamesob>
how welcome are readability improvement & documentation PRs? low-priority distraction or actually welcome? I see a few things in validation.cpp that could be made a bit more readable.
< gmaxwell>
jamesob: they're welcome if that is what they do; but "change the code to my personal style; because I'm not involved with the project enough to distinguish what I'd like with what other contributors would like" wouldn't be, and it's not always easy to tell unless you're heavily involved.
< jamesob>
gotcha. maybe I'll save them for when my intuition is better :)
< gmaxwell>
comments are probably always good, though they take a fair amount of work to review .. since they need to be right. :)
< gmaxwell>
I think that would be a fine change. oh also on these kinds of changes, if they're ones that provably don't change behavior (e.g. stripped object code is the same) that's always helpful.
< jamesob>
gmaxwell does that amount to comparing the checksum of `bitcoind` before and after change?
< jamesob>
probably something a little more granular...
< gmaxwell>
.o of the relevant file is more granular. diffing and objdump of the .o
< jamesob>
cool, thanks
< gmaxwell>
In any case, our threshold for taking improvements like this is lower if it doesn't change the binary. It's also useful to seperate changes into ones that change the binary and ones that don't-- if it doesn't change the binary we only need to review for style. If it changes it we have to worry about subtle mistakes that break the software. :)
< jamesob>
great, good to know. I'm not keen on flooding the PR list with a bunch of [trivial] titles, but I figure if I'm in the neighborhood and can offer some marginal improvement, maybe it's worthwhile.
< gmaxwell>
one thing to do is just rate limit yourself. Do a couple, sleep on it, submit the best of them. Collect feedback, revise the others, submit another later. You can also earn review attention by making an effort to do things that other people want done but don't want to do.
< gmaxwell>
Or by doing some things which everyone reconizes as clearly useful (clarity fixups are not as universally reconized as useful as writing non-trivial tests, for example)