12020-05-17T00:00:04 *** rob01 has quit IRC
22020-05-17T00:00:09 *** proofofkeags has quit IRC
32020-05-17T00:00:43 *** proofofkeags has joined #bitcoin-core-dev
42020-05-17T00:04:54 *** proofofkeags has quit IRC
52020-05-17T00:06:26 *** mol has joined #bitcoin-core-dev
62020-05-17T00:09:36 *** Dean_Guss has joined #bitcoin-core-dev
72020-05-17T00:13:32 <ariard> #proposedmeetingtopic alternative transports support (#18989)
82020-05-17T00:13:33 <gribble> https://github.com/bitcoin/bitcoin/issues/18989 | Towards alternative transports first-class support · Issue #18989 · bitcoin/bitcoin · GitHub
92020-05-17T00:16:20 *** proofofkeags has joined #bitcoin-core-dev
102020-05-17T00:17:16 *** blackjid has joined #bitcoin-core-dev
112020-05-17T00:20:55 *** proofofkeags has quit IRC
122020-05-17T00:20:56 *** mol has quit IRC
132020-05-17T00:21:17 *** mol has joined #bitcoin-core-dev
142020-05-17T00:31:47 *** promag has joined #bitcoin-core-dev
152020-05-17T00:36:15 *** promag has quit IRC
162020-05-17T00:46:46 *** promag has joined #bitcoin-core-dev
172020-05-17T00:51:45 *** promag has quit IRC
182020-05-17T00:56:02 *** luke-jr has quit IRC
192020-05-17T00:56:25 *** proofofk_ has joined #bitcoin-core-dev
202020-05-17T00:57:14 *** Emcy has quit IRC
212020-05-17T01:00:48 *** jonatack_ has joined #bitcoin-core-dev
222020-05-17T01:03:57 *** Emcy has joined #bitcoin-core-dev
232020-05-17T01:04:25 *** jonatack has quit IRC
242020-05-17T01:04:47 *** luke-jr has joined #bitcoin-core-dev
252020-05-17T01:09:22 *** tryphe_ has joined #bitcoin-core-dev
262020-05-17T01:09:53 *** tryphe has quit IRC
272020-05-17T01:13:18 *** geeker has joined #bitcoin-core-dev
282020-05-17T01:18:13 *** geeker has quit IRC
292020-05-17T01:24:11 *** bitcoin-git has joined #bitcoin-core-dev
302020-05-17T01:24:11 <bitcoin-git> [bitcoin] naumenkogs opened pull request #18991: Cache responses to GETADDR to prevent topology leaks (master...2020-05-addr-response-caching) https://github.com/bitcoin/bitcoin/pull/18991
312020-05-17T01:24:17 *** bitcoin-git has left #bitcoin-core-dev
322020-05-17T01:32:40 *** surja795 has quit IRC
332020-05-17T01:38:01 *** mrostecki is now known as mrostecki[m]
342020-05-17T01:45:04 *** s3r6iu has joined #bitcoin-core-dev
352020-05-17T02:03:14 *** s3r6iu has quit IRC
362020-05-17T02:04:38 *** dviola has quit IRC
372020-05-17T02:05:23 *** EagleTM has quit IRC
382020-05-17T02:22:21 *** promag has joined #bitcoin-core-dev
392020-05-17T02:23:33 *** dviola has joined #bitcoin-core-dev
402020-05-17T02:24:03 *** dviola has left #bitcoin-core-dev
412020-05-17T02:24:46 *** dviola has joined #bitcoin-core-dev
422020-05-17T02:27:10 *** promag has quit IRC
432020-05-17T02:31:47 *** BlueMatt has quit IRC
442020-05-17T02:33:26 *** surja795 has joined #bitcoin-core-dev
452020-05-17T02:37:17 *** geeker has joined #bitcoin-core-dev
462020-05-17T02:39:56 *** proofofk_ has quit IRC
472020-05-17T02:41:36 *** geeker has quit IRC
482020-05-17T02:43:07 *** BlueMatt has joined #bitcoin-core-dev
492020-05-17T02:45:34 *** dviola has quit IRC
502020-05-17T02:45:41 *** ubuntu has joined #bitcoin-core-dev
512020-05-17T02:45:47 *** ubuntu has quit IRC
522020-05-17T02:46:23 *** mrostecki has joined #bitcoin-core-dev
532020-05-17T02:46:46 *** dviola has joined #bitcoin-core-dev
542020-05-17T02:48:28 *** dviola has quit IRC
552020-05-17T03:00:02 *** blackjid has quit IRC
562020-05-17T03:09:57 *** bitcoin-git has joined #bitcoin-core-dev
572020-05-17T03:09:58 <bitcoin-git> [bitcoin] 10xcryptodev opened pull request #18992: qt: add sign and verify message support to AddressBookPage (master...202005-sign-message-ui) https://github.com/bitcoin/bitcoin/pull/18992
582020-05-17T03:09:59 *** bitcoin-git has left #bitcoin-core-dev
592020-05-17T03:19:29 *** promag has joined #bitcoin-core-dev
602020-05-17T03:19:46 *** chaosagent has joined #bitcoin-core-dev
612020-05-17T03:31:40 *** IGHOR has quit IRC
622020-05-17T03:31:44 *** promag has joined #bitcoin-core-dev
632020-05-17T03:36:10 *** promag has quit IRC
642020-05-17T03:36:19 *** Relis has quit IRC
652020-05-17T03:40:53 *** IGHOR has joined #bitcoin-core-dev
662020-05-17T03:44:24 *** Relis has joined #bitcoin-core-dev
672020-05-17T03:44:59 *** geeker has joined #bitcoin-core-dev
682020-05-17T03:52:03 *** surja795 has quit IRC
692020-05-17T03:53:33 *** surja795 has joined #bitcoin-core-dev
702020-05-17T04:00:39 *** Relis has quit IRC
712020-05-17T04:06:53 *** surja795 has quit IRC
722020-05-17T04:16:29 *** sipa has quit IRC
732020-05-17T04:20:02 *** vasild_ has joined #bitcoin-core-dev
742020-05-17T04:23:03 *** vasild has quit IRC
752020-05-17T04:23:04 *** vasild_ is now known as vasild
762020-05-17T04:32:05 *** sipa has joined #bitcoin-core-dev
772020-05-17T04:34:45 *** ghost43 has quit IRC
782020-05-17T04:35:02 *** ghost43 has joined #bitcoin-core-dev
792020-05-17T04:58:37 *** bitcoin-git has joined #bitcoin-core-dev
802020-05-17T04:58:37 <bitcoin-git> [bitcoin] 10xcryptodev opened pull request #18993: qt: increase console command max lenght (master...202005-increase-console-command) https://github.com/bitcoin/bitcoin/pull/18993
812020-05-17T04:58:38 *** bitcoin-git has left #bitcoin-core-dev
822020-05-17T05:04:21 *** tryphe_ is now known as tryphe
832020-05-17T05:06:55 <tryphe> does a tACK imply a concept ack? or it is simply a confirmation that the commit works, regardless of conceptual opinion (suppose you agree with the concept but don't know whether to agree with the approach taken or not)?
842020-05-17T05:07:54 <yevaud> my reading is that you wouldn't test something you don't conceptually agree with.
852020-05-17T05:08:57 <tryphe> makes sense
862020-05-17T05:09:20 <yevaud> that may not be correct, but I'm not sure how to understand it otherwise, as you've pointed out.
872020-05-17T05:11:10 *** Highway62 has joined #bitcoin-core-dev
882020-05-17T05:11:50 *** Highway62 has quit IRC
892020-05-17T05:11:56 *** Highway61 has quit IRC
902020-05-17T05:12:57 <tryphe> my feeling was that maybe you'd test it just to test it because it's a good concept, without being able to weigh the tradeoffs of what's possible with another implementation of the same concept
912020-05-17T05:13:40 <tryphe> and maybe you'd concept ack it if you had gone through implementations of that concept in your head
922020-05-17T05:14:04 <tryphe> (or maybe that would be something different like implementation ack)
932020-05-17T05:15:54 <tryphe> and maybe code review is more of an implementation ack
942020-05-17T05:34:01 *** mol_ has joined #bitcoin-core-dev
952020-05-17T05:37:01 *** mol has quit IRC
962020-05-17T05:54:33 *** bitcoin-git has joined #bitcoin-core-dev
972020-05-17T05:54:33 <bitcoin-git> [bitcoin] practicalswift opened pull request #18994: tests: Add fuzzing harnesses for functions in script/ (master...fuzzers-script-slash) https://github.com/bitcoin/bitcoin/pull/18994
982020-05-17T05:54:34 *** bitcoin-git has left #bitcoin-core-dev
992020-05-17T06:00:02 *** chaosagent has quit IRC
1002020-05-17T06:05:41 <sipa> tryphe: sometimes people comment "code review ack" to be specific about the fact that the code looks good, but they don't have much of an opinion on the concept
1012020-05-17T06:07:31 *** ctrlbreak_MAD has joined #bitcoin-core-dev
1022020-05-17T06:07:52 *** Aaronvan_ has joined #bitcoin-core-dev
1032020-05-17T06:10:57 *** AaronvanW has quit IRC
1042020-05-17T06:10:57 *** ctrlbreak has quit IRC
1052020-05-17T06:18:54 *** afk11` has quit IRC
1062020-05-17T06:19:40 *** afk11` has joined #bitcoin-core-dev
1072020-05-17T06:22:06 *** ramsey1 has joined #bitcoin-core-dev
1082020-05-17T06:28:13 *** manantial has joined #bitcoin-core-dev
1092020-05-17T06:29:52 *** dfmb_ has joined #bitcoin-core-dev
1102020-05-17T06:29:57 *** dfmb__ has joined #bitcoin-core-dev
1112020-05-17T06:30:35 *** dfmb__ has quit IRC
1122020-05-17T06:30:35 *** dfmb_ has quit IRC
1132020-05-17T06:36:08 <tryphe> sipa, ahh, that makes sense then, i guess i had assumed those code review acks were concept acks in a sense, but indeed it's nice that code review can complete without putting concept ack pressure on the reviewers
1142020-05-17T06:55:33 <tryphe> on another note: shouldn't there be a github tag called "needs testing", or are testers in high supply and usually not necessary??
1152020-05-17T06:55:51 <tryphe> err, multiple question marks not intended, sorry
1162020-05-17T06:56:26 *** dfmb_ has joined #bitcoin-core-dev
1172020-05-17T06:57:17 <tryphe> usually not necessary/usually not necessary enough to need a tag*
1182020-05-17T06:58:55 <tryphe> or is that what "review club" is?
1192020-05-17T07:04:40 *** dfmbbtc has joined #bitcoin-core-dev
1202020-05-17T07:06:10 *** jonatack_ has quit IRC
1212020-05-17T07:07:27 *** dfmb_ has quit IRC
1222020-05-17T07:19:07 *** dfmb_btc has joined #bitcoin-core-dev
1232020-05-17T07:22:50 *** Talkless has joined #bitcoin-core-dev
1242020-05-17T07:22:56 *** dfmbbtc has quit IRC
1252020-05-17T07:26:42 *** marcoagner has joined #bitcoin-core-dev
1262020-05-17T07:35:05 *** Talkless has joined #bitcoin-core-dev
1272020-05-17T07:35:50 *** Talkless has joined #bitcoin-core-dev
1282020-05-17T07:36:40 *** Talkless has joined #bitcoin-core-dev
1292020-05-17T07:40:59 *** Dean_Guss has quit IRC
1302020-05-17T07:41:20 *** Dean_Guss has joined #bitcoin-core-dev
1312020-05-17T07:42:25 *** jonatack_ has joined #bitcoin-core-dev
1322020-05-17T07:42:41 *** tmoc has quit IRC
1332020-05-17T07:43:03 *** jonatack_ has quit IRC
1342020-05-17T07:43:13 *** jonatack has joined #bitcoin-core-dev
1352020-05-17T08:17:05 *** promag has joined #bitcoin-core-dev
1362020-05-17T08:18:34 *** mol_ has quit IRC
1372020-05-17T08:21:45 *** promag has quit IRC
1382020-05-17T08:31:05 *** geeker has quit IRC
1392020-05-17T08:44:30 *** per has quit IRC
1402020-05-17T08:44:44 *** per has joined #bitcoin-core-dev
1412020-05-17T08:50:46 *** EagleTM has joined #bitcoin-core-dev
1422020-05-17T08:51:13 *** surja795 has joined #bitcoin-core-dev
1432020-05-17T08:52:10 *** geeker has joined #bitcoin-core-dev
1442020-05-17T08:55:35 *** surja795 has quit IRC
1452020-05-17T08:56:34 *** geeker has quit IRC
1462020-05-17T08:58:07 *** emilengler has joined #bitcoin-core-dev
1472020-05-17T09:00:02 *** ramsey1 has quit IRC
1482020-05-17T09:03:46 *** shigeya has quit IRC
1492020-05-17T09:04:05 *** shigeya has joined #bitcoin-core-dev
1502020-05-17T09:07:56 *** AaronvanW has joined #bitcoin-core-dev
1512020-05-17T09:10:49 *** Aaronvan_ has quit IRC
1522020-05-17T09:12:16 *** Pavlenex has joined #bitcoin-core-dev
1532020-05-17T09:20:45 *** Alphi has joined #bitcoin-core-dev
1542020-05-17T09:37:03 *** emilengler has quit IRC
1552020-05-17T09:47:56 *** emilengler has joined #bitcoin-core-dev
1562020-05-17T10:01:30 <jonatack> tryphe: i think the review club is to help contributors learn about the review process and participate in it
1572020-05-17T10:03:00 *** promag has joined #bitcoin-core-dev
1582020-05-17T10:03:16 <jonatack> as for the need for a testing label: ACKs are normally followed by a description of how the reviewer did the review, including testing (https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#code-review)
1592020-05-17T10:05:16 <jonatack> (though that somewhat newer guideline isn't necessarily followed). I don't think we have an oversupply of testing and reviewing.
1602020-05-17T10:05:24 *** Jaycee66Ryan has joined #bitcoin-core-dev
1612020-05-17T10:10:35 *** Jaycee66Ryan has quit IRC
1622020-05-17T10:13:15 <jonatack> sipa: good point, i added your code review ack description to https://jonatack.github.io/articles/how-to-review-pull-requests-in-bitcoin-core#peer-review
1632020-05-17T10:15:47 *** promag_ has joined #bitcoin-core-dev
1642020-05-17T10:21:04 *** jonatack has quit IRC
1652020-05-17T10:21:04 *** mol has joined #bitcoin-core-dev
1662020-05-17T10:41:20 *** jonatack has joined #bitcoin-core-dev
1672020-05-17T11:11:37 <meshcollider> In my opinion, code review ACK is just the more descriptive version of utACK, and that any form of ACK (including tACK) is implicitly a concept ACK unless stated otherwise
1682020-05-17T11:17:16 *** belcher has quit IRC
1692020-05-17T11:19:23 *** Dean_Guss has quit IRC
1702020-05-17T11:19:33 *** promag_ has quit IRC
1712020-05-17T11:21:17 *** rh0nj has quit IRC
1722020-05-17T11:22:07 *** rh0nj has joined #bitcoin-core-dev
1732020-05-17T11:24:58 *** geeker has joined #bitcoin-core-dev
1742020-05-17T11:25:57 *** promag has quit IRC
1752020-05-17T11:26:33 *** promag has joined #bitcoin-core-dev
1762020-05-17T11:33:53 <jonatack> meshcollider: this is how i have been using it as well. the more specific version had not occurred to me, but it strikes me as more useful. if i'm not the only one who read it this way then it may be useful to disambiguate it.
1772020-05-17T11:36:38 *** emilengler has quit IRC
1782020-05-17T11:43:11 *** surja795 has joined #bitcoin-core-dev
1792020-05-17T12:00:02 *** Alphi has quit IRC
1802020-05-17T12:14:30 *** Pavlenex has quit IRC
1812020-05-17T12:21:14 *** smibarber has joined #bitcoin-core-dev
1822020-05-17T12:21:40 *** bitcoin-git has joined #bitcoin-core-dev
1832020-05-17T12:21:41 <bitcoin-git> [bitcoin] MarcoFalke pushed 5 commits to master: https://github.com/bitcoin/bitcoin/compare/f8123d483caa...dc5333d31f28
1842020-05-17T12:21:41 <bitcoin-git> bitcoin/master c0bbf81 practicalswift: tests: Fill fuzzing coverage gaps for functions in primitives/block.h
1852020-05-17T12:21:42 <bitcoin-git> bitcoin/master b74f3d6 practicalswift: tests: Fill fuzzing coverage gaps for functions in consensus/validation.h
1862020-05-17T12:21:42 <bitcoin-git> bitcoin/master fb559c1 practicalswift: tests: Fill fuzzing coverage gaps for functions in util/translation.h
1872020-05-17T12:21:52 *** bitcoin-git has left #bitcoin-core-dev
1882020-05-17T12:22:10 *** bitcoin-git has joined #bitcoin-core-dev
1892020-05-17T12:22:11 <bitcoin-git> [bitcoin] MarcoFalke merged pull request #18938: tests: Fill fuzzing coverage gaps for functions in consensus/validation.h, primitives/block.h and util/translation.h (master...fuzzers-misc-5) https://github.com/bitcoin/bitcoin/pull/18938
1902020-05-17T12:22:11 *** bitcoin-git has left #bitcoin-core-dev
1912020-05-17T12:25:42 *** promag_ has joined #bitcoin-core-dev
1922020-05-17T12:28:01 *** belcher has joined #bitcoin-core-dev
1932020-05-17T12:29:54 *** promag_ has quit IRC
1942020-05-17T12:49:19 *** Technoprenerd has quit IRC
1952020-05-17T12:58:54 *** Chris_Stewart_5 has joined #bitcoin-core-dev
1962020-05-17T13:05:24 <michaelfolkson> jonatack: This isn't the guidance in the CONTRIBUTING doc. https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#code-review
1972020-05-17T13:06:07 <michaelfolkson> I thought the whole motivation for the change in wording last year was to make it more standardized and less convoluted.
1982020-05-17T13:07:44 <jonatack> michaelfolkson: this is what i meant above with "(though that somewhat newer guideline isn't necessarily followed)"
1992020-05-17T13:08:28 <michaelfolkson> Well they should be right? What is the point of guidelines if no one follows them?
2002020-05-17T13:08:55 <jonatack> michaelfolkson: we're discussing what is seen in practice... "code review ack" is used frequently
2012020-05-17T13:09:15 <michaelfolkson> If we want to get BitcoinACKs back up and other tools/analytics we need people to understand the guidelines and follow those guidelines
2022020-05-17T13:09:33 <jonatack> michaelfolkson: this is open source...
2032020-05-17T13:09:47 <michaelfolkson> Ok let's ditch the guidelines then
2042020-05-17T13:10:53 <michaelfolkson> I don't think guidance docs like the one you put together should be encouraging things that aren't in the guidelines
2052020-05-17T13:13:43 <jonatack> i update it often to reflect actual practice, it's not intended to be a copy of contributing.md. you are free to write your own document. this is open source.
2062020-05-17T13:13:44 <michaelfolkson> That makes things really confusing if two different docs tell you to do two different things
2072020-05-17T13:15:10 <michaelfolkson> Perhaps ditch the style guidelines too if this is open source. I don't mean to be flippant but "this is open source so feel free not to follow guidelines" seems bizarre to me
2082020-05-17T13:18:24 <jonatack> observing and discussing actual practice is different, in my view, from proposing to ditch guidelines, which can still be helpful
2092020-05-17T13:18:54 <jonatack> i'm not really interested in debating that
2102020-05-17T13:21:00 <michaelfolkson> As long you appreciate downsides. You can't one day talk about why BitcoinACKs doesn't effectively work and the next day encourage flexibility around the review wordings people use
2112020-05-17T13:21:07 *** thomasb06 has joined #bitcoin-core-dev
2122020-05-17T13:21:49 <michaelfolkson> I don't know. I don't want to discuss it either but I am more confused after this conversation than I was before it started. Let's leave it
2132020-05-17T13:22:19 *** Relis has joined #bitcoin-core-dev
2142020-05-17T13:22:23 *** sipa has quit IRC
2152020-05-17T13:23:27 <jonatack> i'm doing neither of those things, i think? bitcoinacks could be updated occasionally when practices evolve. i'm not advocating about current practices, simply looking to understand them
2162020-05-17T13:29:36 *** sipa has joined #bitcoin-core-dev
2172020-05-17T13:33:43 *** Relis has quit IRC
2182020-05-17T14:05:30 *** owowo has quit IRC
2192020-05-17T14:08:02 *** Relis has joined #bitcoin-core-dev
2202020-05-17T14:10:08 *** owowo has joined #bitcoin-core-dev
2212020-05-17T14:10:08 *** owowo has joined #bitcoin-core-dev
2222020-05-17T14:16:19 *** Guyver2 has joined #bitcoin-core-dev
2232020-05-17T14:21:55 *** bitdex has quit IRC
2242020-05-17T14:22:27 *** bitdex has joined #bitcoin-core-dev
2252020-05-17T14:26:37 *** Highway61 has joined #bitcoin-core-dev
2262020-05-17T14:39:37 *** Pavlenex has joined #bitcoin-core-dev
2272020-05-17T14:41:33 *** bitcoin-git has joined #bitcoin-core-dev
2282020-05-17T14:41:33 <bitcoin-git> [bitcoin] MarcoFalke opened pull request #18996: net: Remove un-actionable TODO (master...2005-netNoTodo) https://github.com/bitcoin/bitcoin/pull/18996
2292020-05-17T14:41:36 *** bitcoin-git has left #bitcoin-core-dev
2302020-05-17T14:44:48 *** mol_ has joined #bitcoin-core-dev
2312020-05-17T14:47:26 *** mol has quit IRC
2322020-05-17T14:50:09 *** proofofkeags has joined #bitcoin-core-dev
2332020-05-17T14:56:43 *** luke-jr has quit IRC
2342020-05-17T14:59:20 *** geeker has quit IRC
2352020-05-17T14:59:51 *** bitcoin-git has joined #bitcoin-core-dev
2362020-05-17T14:59:51 <bitcoin-git> [bitcoin] MarcoFalke opened pull request #18997: gui: Remove un-actionable TODO (master...2005-guiNoTodo) https://github.com/bitcoin/bitcoin/pull/18997
2372020-05-17T15:00:00 *** bitcoin-git has left #bitcoin-core-dev
2382020-05-17T15:00:01 *** smibarber has quit IRC
2392020-05-17T15:02:34 *** proofofkeags has quit IRC
2402020-05-17T15:03:07 *** proofofkeags has joined #bitcoin-core-dev
2412020-05-17T15:07:30 *** proofofkeags has quit IRC
2422020-05-17T15:07:42 *** proofofkeags has joined #bitcoin-core-dev
2432020-05-17T15:09:21 *** geeker has joined #bitcoin-core-dev
2442020-05-17T15:12:32 *** proofofkeags has quit IRC
2452020-05-17T15:13:05 *** proofofkeags has joined #bitcoin-core-dev
2462020-05-17T15:13:54 *** geeker has quit IRC
2472020-05-17T15:14:19 *** proofofkeags has quit IRC
2482020-05-17T15:21:58 *** mattl1 has joined #bitcoin-core-dev
2492020-05-17T15:35:13 *** emilengler has joined #bitcoin-core-dev
2502020-05-17T16:00:58 *** geeker has joined #bitcoin-core-dev
2512020-05-17T16:05:38 *** geeker has quit IRC
2522020-05-17T16:13:15 *** Relis has quit IRC
2532020-05-17T16:20:04 *** vasild_ has joined #bitcoin-core-dev
2542020-05-17T16:21:36 *** Highway62 has joined #bitcoin-core-dev
2552020-05-17T16:21:47 *** Highway61 has quit IRC
2562020-05-17T16:21:47 *** Highway62 is now known as Highway61
2572020-05-17T16:23:23 *** vasild has quit IRC
2582020-05-17T16:23:24 *** vasild_ is now known as vasild
2592020-05-17T16:26:17 *** Pavlenex has quit IRC
2602020-05-17T16:27:08 *** Relis has joined #bitcoin-core-dev
2612020-05-17T16:38:48 <tryphe> jonatack, meshcollider, thanks!
2622020-05-17T16:43:43 *** bitcoin-git has joined #bitcoin-core-dev
2632020-05-17T16:43:43 <bitcoin-git> [bitcoin] MarcoFalke opened pull request #18999: 2005 log rpc password (master...2005-logRpcPassword) https://github.com/bitcoin/bitcoin/pull/18999
2642020-05-17T16:43:45 *** bitcoin-git has left #bitcoin-core-dev
2652020-05-17T16:45:16 <tryphe> another question about review though. is there a general need for tested ACKs other than from reviewers? i guess what i mean is, is testing and reviewing seen as more mutually exclusive, where everyone testing a PR is seen beneficial, beyond reviewer testing?
2662020-05-17T16:46:56 <michaelfolkson> On your previous question tryphe I would say follow the guidelines as much as possible https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#code-review
2672020-05-17T16:46:58 <instagibbs> tryphe, code reviewing and testing are two separate things generally
2682020-05-17T16:47:43 <instagibbs> for instance some PRs I just don't have time/interest/knowledge to review the code well, but I know how to test how users are expected to use it, try to trip it up, etc. Both are valuable.
2692020-05-17T16:48:07 <sipa> i'd say that the need for manual testing goes down the more testable (through unit or functional or fuzz tests) it is
2702020-05-17T16:48:23 <sipa> and the latter is certainly preferable
2712020-05-17T16:49:13 <sipa> so the distinction between "ack, tested" and "ack, didn't test anything myself but the included tests look sufficient"
2722020-05-17T16:49:18 <sipa> is not always so big
2732020-05-17T16:49:21 <tryphe> michaelfolkson, yep, i took a look, but i wasn't sure if tACKS were inherently concept ACKs or not, or if both should be done
2742020-05-17T16:50:23 <tryphe> i guess maybe that should be added? it seems like newer people might get thrown off by that
2752020-05-17T16:50:41 <michaelfolkson> tryphe: I think generally if you are taking the time to review the code you are a Concept ACK or you think the Concept ACK has consensus. Otherwise why review the code?
2762020-05-17T16:51:04 <michaelfolkson> tryphe: But you can Concept NACK and ACK a commit in the same comment if you are in that situation
2772020-05-17T16:51:18 <sipa> indeed
2782020-05-17T16:51:28 <sipa> but you may be indifferent about the concept
2792020-05-17T16:51:43 <tryphe> michaelfolkson, suppose you are feeling neutral about the concept, reviewed the code, but want to leave the concept ack to other people to decide
2802020-05-17T16:51:48 <tryphe> yeah what sipa said
2812020-05-17T16:52:06 <sipa> "Code review ACK <commitid>, unsure about the concept"
2822020-05-17T16:52:37 <sipa> You're always welcome to elaborate more on your opinion.
2832020-05-17T16:53:00 <michaelfolkson> The guideline is ACK <commitid> rather than Code review ACK <commitid>
2842020-05-17T16:53:30 <michaelfolkson> Obviously you can write whatever you want after ACK <commitid> to explain in greater detail exactly what you have reviewed
2852020-05-17T16:53:59 <michaelfolkson> That's the current guideline (as I can make out)
2862020-05-17T16:55:14 <tryphe> that makes sense, thanks
2872020-05-17T16:55:20 <sipa> for example other useful things to add are "verified move only" if the PR includes move-only commits, and things like "thought hard about how the change X could break Y but didn't find any"
2882020-05-17T16:56:16 <tryphe> i had found myself going through a bunch of historical commits to try and find a theme but it seems like everyone has their own preference of style, i think that might deter newer folks from jumping right in, unsure about what they've seen vs. what they interpret the guidelines as
2892020-05-17T16:56:34 <tryphe> i mean, just as an observation
2902020-05-17T16:57:21 <sipa> yeah, that's certainly possible
2912020-05-17T16:57:25 <michaelfolkson> Maybe. I am happy to be one of those annoying people who badger people to follow the guidelines ;)
2922020-05-17T16:57:46 <michaelfolkson> We certainly need those richer comments that sipa lays out too. Hopefully we can get best of both worlds
2932020-05-17T16:57:56 <tryphe> not nitpicking the guidelines or anything, i just wish it was easier to parse i guess, but can't really suggest any improvments
2942020-05-17T16:58:24 <jonatack> i think this was the last time recently where i reviewed the code before the concept ack https://github.com/bitcoin/bitcoin/pull/18962#issuecomment-627635800
2952020-05-17T16:58:25 <michaelfolkson> It is good feedback. I think the guidelines are good. Just need to try to get people to follow them I think
2962020-05-17T16:59:13 <jonatack> when in doubt adding context doesn't hurt; the shortcuts seem popular because they are just handy
2972020-05-17T17:01:00 <tryphe> i see, that makes sense
2982020-05-17T17:01:14 *** Pavlenex has joined #bitcoin-core-dev
2992020-05-17T17:01:43 <tryphe> i guess my concern is, to the casual observer or someone who might be a potential tester, seeing the shortcut acks might throw them through a loop about the whole process
3002020-05-17T17:02:22 <tryphe> even if they had read the guidelines after, i mean
3012020-05-17T17:04:19 <sipa> i think from a new contributor it's even more advisable to be verbose in review comments
3022020-05-17T17:04:27 *** Highway61 has quit IRC
3032020-05-17T17:10:06 <michaelfolkson> Makes sense. A long term contributor ACKing a commit without any comment has value. But if a new contributor does the same it is unclear what exactly they have reviewed and if they have truly understood the change
3042020-05-17T17:10:42 <sipa> exactly
3052020-05-17T17:10:45 <tryphe> ahh yeah
3062020-05-17T17:11:32 <tryphe> as someone who has looked over a lot of PRs but never commented much, i always feel like verbose conversation was sort of unwelcome and would clog up the comment feed with chatter instead of acks, but i guess that's completely wrong
3072020-05-17T17:13:14 <sipa> i think the most disrupting thing to a PR is getting a multitude of low level/nits/code style comments, while it's very unclear if a PR is desirable as a concept
3082020-05-17T17:14:09 <michaelfolkson> Personally I have been quite conservative on when I comment. John Newbery has encouraged me to comment more on PRs having participated in a lot of PR review clubs.
3092020-05-17T17:14:55 <michaelfolkson> It is a balance though I think. Often I am trying to understand the conceptual change at a time when it clearly already has consensus on the Concept ACK and the discussion has moved onto the code review
3102020-05-17T17:15:36 <michaelfolkson> Definitely do a lot of PR review clubs tryphe and it will start to become clearer where you can add value
3112020-05-17T17:16:23 *** mrostecki has quit IRC
3122020-05-17T17:16:31 *** Dean_Guss has joined #bitcoin-core-dev
3132020-05-17T17:17:29 <tryphe> thanks. i think i might do that!
3142020-05-17T17:25:33 <tryphe> thanks. i think i might do that!
3152020-05-17T17:25:36 <tryphe> oops
3162020-05-17T17:25:39 <tryphe> when getting into bitcoin PR review i feel like there's this -almost- unscalable mountain in front of you; semi-scalable in terms of "things that i can actually review without much historical bitcoin knowlegde" (NATPMP for example, that was easy, but almost straightforward enough that it requires no comments), but also unscalable parts in terms of other PRs where the motivation was so ingenius that trying to review it almost seems like a
3172020-05-17T17:25:39 <tryphe> fallacy (maybe i understand the motivation conceptually but trying to constructively add to conversation is not possible)
3182020-05-17T17:26:27 <tryphe> i guess the almost unscable part becomes scalable over time with much more PRs reviewed though, but to me that's the biggest detterent to newcomers
3192020-05-17T17:26:58 <tryphe> hard to constructively comment on low hanging fruit (because it's easy), but overwhelmed when trying to push a boulder, so to speak
3202020-05-17T17:28:31 *** mrostecki has joined #bitcoin-core-dev
3212020-05-17T17:30:53 <tryphe> i guess it seems like there's a lack of middle ground, just a huge disparity between simple changes and huge ones. observationally speaking, anyways.
3222020-05-17T17:30:55 <jonatack> re-updated today's update with the suggestions from sipa: https://github.com/jonatack/jonatack.github.io/commit/e050fdd
3232020-05-17T17:32:15 <michaelfolkson> Yeah I think that is fair tryphe. It is not easy. But ACKing the more basic PRs and attending the PR review club to understand the more complex ones is the way to go
3242020-05-17T17:33:59 <tryphe> michaelfolkson, thanks for that, i'll definitely give it a go. is the review club new, btw? it seems like it wasn't around even a few years ago, or maybe i'm just ignorant
3252020-05-17T17:34:49 <sipa> tryphe: it just celebrated its 1 year anniversary
3262020-05-17T17:34:54 <jonatack> tryphe: jnewbery launched it began a year ago. it's sort of like an online version of the chaincode labs seminars.
3272020-05-17T17:35:05 <tryphe> that's pretty cool
3282020-05-17T17:35:25 <tryphe> there's such a large following now it almost seems like it existed longer
3292020-05-17T17:35:34 <michaelfolkson> jonatack: Looks good apart from code review ACK. As I said before the guideline is ACK <commitid> and then comment e.g. Reviewed the code.
3302020-05-17T17:36:05 <michaelfolkson> We have no hope of people following guidelines if guidelines aren't clear and inconsistent between documents
3312020-05-17T17:36:09 <tryphe> existed/had to exist*
3322020-05-17T17:36:45 <jonatack> michaelfolkson: thanks for reviewing, will look at it tomorrow with fresh eyes
3332020-05-17T17:37:02 <michaelfolkson> At this point your doc is the most useful doc out there. Important to get it right in my view
3342020-05-17T17:39:54 <jonatack> ð
3352020-05-17T17:50:23 *** jb55 has quit IRC
3362020-05-17T17:53:50 *** jb55 has joined #bitcoin-core-dev
3372020-05-17T17:59:20 <tryphe> what do you guys think of the PR-comment reaction emojis on github? is it generally useless to thumbs up the first message in a conversation, but okay to thumbs up a comment that you agree with but don't care to comment on?
3382020-05-17T17:59:41 <tryphe> or is it better to just reply?
3392020-05-17T18:00:02 *** mattl1 has quit IRC
3402020-05-17T18:00:32 <tryphe> i was just curious if anyone used the emojis as a metric or if it's just purely visual in terms of review
3412020-05-17T18:02:44 <tryphe> ie. maybe sometimes emojis would be preferred, or maybe it's just seen as a dumb idea
3422020-05-17T18:05:14 *** Victorsueca has quit IRC
3432020-05-17T18:06:12 <tryphe> in some ways i think it's hard to parse if someone gave an edited comment a thumbs up, because it's hard to know which version of the comment they approved of, but otoh replying to a full copy of a message every time seems spammy
3442020-05-17T18:08:28 <jonatack> tryphe: on github projects with a small number of collaborators i think they are fun; on more widely-watched ones like bitcoin core i tend to not use them to avoid it turning into a social media or slack feed. using an emoji inside a comment, sure. worked more ideas into the doc from all three of you, thanks!
3452020-05-17T18:09:59 <tryphe> lol, good point :)
3462020-05-17T18:10:27 *** tmoc has joined #bitcoin-core-dev
3472020-05-17T18:10:49 <gleb> tryphe: I like emojis and use them for different purposes. But at the same time I don't assume that everybody looks at them. So if I really need to say that I agree, I'd rather make an explicit message.
3482020-05-17T18:10:59 *** Victorsueca has joined #bitcoin-core-dev
3492020-05-17T18:11:38 <michaelfolkson> tryphe: Emojis definitely aren't used as a formal metric. I find them useful to see whether people are enthusiastic or not about a comment without them needing to add unnecessary comments
3502020-05-17T18:12:35 <michaelfolkson> tryphe: Agree with gleb. If you have something important to say it needs a comment. If you don't have much to say but like/dislike a comment some people (e.g. gleb and me) take notice ;)
3512020-05-17T18:13:01 <jonatack> hm, i do use the heart emoji to say thanks to people sometimes for reviewing
3522020-05-17T18:14:01 <tryphe> ahh, good point. yeah i think in some sense emojis are somewhat redundant other than a simple "i agree but no comment needed"
3532020-05-17T18:14:21 <michaelfolkson> I only realized recently when Marco commented that too many unnecessary comments is a problem. Makes reviewers life more difficult because GitHub starts hiding important review comments
3542020-05-17T18:15:13 <michaelfolkson> So that makes emojis useful. You don't want lots of "Yes I agree with the above comment" statements
3552020-05-17T18:15:52 <sipa> Unfortunately, emoji are also mostly anonymous.
3562020-05-17T18:16:12 <michaelfolkson> No you can hover over it and see who posted the emoji?
3572020-05-17T18:16:23 <sipa> sure
3582020-05-17T18:16:26 *** bitdex has quit IRC
3592020-05-17T18:16:28 <sipa> that's why i say mostly
3602020-05-17T18:16:43 <sipa> but it's not clear at a first glance who liked/agreed/disliked
3612020-05-17T18:17:02 <sipa> and when a comment gets brigaded by random people because someone linked it on social media, it becomes entirely worthless
3622020-05-17T18:17:17 *** bitdex has joined #bitcoin-core-dev
3632020-05-17T18:18:15 <michaelfolkson> Yup. Definitely flawed but generally maintains some limited value in most cases
3642020-05-17T18:18:23 <tryphe> yeah, after a certain threshold it just becomes "alice, bob, jane, and X more reacted"
3652020-05-17T18:20:06 *** geeker has joined #bitcoin-core-dev
3662020-05-17T18:22:01 *** Bjarki has joined #bitcoin-core-dev
3672020-05-17T18:22:17 <tryphe> i think in the future if you had a historical archive of PRs the "X more reacted" might pollute things a little bit, because you don't get to see all participants
3682020-05-17T18:22:40 <tryphe> and maybe some reviewers would get washed out by mass emoji-ers
3692020-05-17T18:24:33 *** geeker has quit IRC
3702020-05-17T18:24:36 *** Talkless has quit IRC
3712020-05-17T18:40:03 *** surja795 has quit IRC
3722020-05-17T18:40:15 *** sipa has quit IRC
3732020-05-17T18:51:00 *** sipa has joined #bitcoin-core-dev
3742020-05-17T18:52:44 *** Henry151_ is now known as Henry151
3752020-05-17T18:56:05 *** surja795 has joined #bitcoin-core-dev
3762020-05-17T19:02:43 *** EagleTM has quit IRC
3772020-05-17T19:06:42 *** mdunnio has joined #bitcoin-core-dev
3782020-05-17T19:11:00 *** mdunnio has quit IRC
3792020-05-17T19:30:10 *** luke-jr has joined #bitcoin-core-dev
3802020-05-17T19:30:44 *** vasild has quit IRC
3812020-05-17T19:45:00 *** promag_ has joined #bitcoin-core-dev
3822020-05-17T19:58:10 *** Highway61 has joined #bitcoin-core-dev
3832020-05-17T20:03:09 *** thomasb06 has left #bitcoin-core-dev
3842020-05-17T20:05:56 *** vasild has joined #bitcoin-core-dev
3852020-05-17T20:14:23 *** EagleTM has joined #bitcoin-core-dev
3862020-05-17T20:21:05 *** Emcy has quit IRC
3872020-05-17T20:22:39 *** geeker has joined #bitcoin-core-dev
3882020-05-17T20:27:07 *** geeker has quit IRC
3892020-05-17T20:39:06 *** emilengler has quit IRC
3902020-05-17T20:43:54 *** promag_ has quit IRC
3912020-05-17T20:50:12 *** Pavlenex has quit IRC
3922020-05-17T20:59:01 *** roconnor has joined #bitcoin-core-dev
3932020-05-17T21:00:02 *** Bjarki has quit IRC
3942020-05-17T21:19:52 *** IOMonster1 has joined #bitcoin-core-dev
3952020-05-17T21:31:06 *** promag_ has joined #bitcoin-core-dev
3962020-05-17T21:35:36 *** manantial has quit IRC
3972020-05-17T21:39:47 *** bitcoin-git has joined #bitcoin-core-dev
3982020-05-17T21:39:48 <bitcoin-git> [bitcoin] 10xcryptodev opened pull request #19001: qt: bugfix unsupported QLocale languages (master...202005-bugfix-qlocale) https://github.com/bitcoin/bitcoin/pull/19001
3992020-05-17T21:39:49 *** bitcoin-git has left #bitcoin-core-dev
4002020-05-17T21:45:06 *** geeker has joined #bitcoin-core-dev
4012020-05-17T21:49:40 *** geeker has quit IRC
4022020-05-17T21:50:06 *** inoor has joined #bitcoin-core-dev
4032020-05-17T21:50:27 *** inoor has left #bitcoin-core-dev
4042020-05-17T21:56:36 *** dfmb_btc has quit IRC
4052020-05-17T22:09:11 *** Chris_Stewart_5 has quit IRC
4062020-05-17T22:11:35 *** Guyver2 has quit IRC
4072020-05-17T22:17:41 *** promag_ has quit IRC
4082020-05-17T22:19:48 *** promag_ has joined #bitcoin-core-dev
4092020-05-17T22:22:14 *** Chris_Stewart_5 has joined #bitcoin-core-dev
4102020-05-17T22:26:53 *** promag_ has quit IRC
4112020-05-17T22:31:51 *** bitcoin-git has joined #bitcoin-core-dev
4122020-05-17T22:31:51 <bitcoin-git> [bitcoin] sipa closed pull request #18977: [DONTMERGE] Test ranked_index (master...202005_try_ranked) https://github.com/bitcoin/bitcoin/pull/18977
4132020-05-17T22:31:53 *** bitcoin-git has left #bitcoin-core-dev
4142020-05-17T22:34:07 *** marcoagner has quit IRC
4152020-05-17T22:39:44 *** dfmb_ has joined #bitcoin-core-dev
4162020-05-17T23:01:52 *** go121212 has joined #bitcoin-core-dev
4172020-05-17T23:02:27 *** dfmb_ has quit IRC
4182020-05-17T23:04:53 *** go11111111111 has quit IRC
4192020-05-17T23:06:35 *** shesek has joined #bitcoin-core-dev
4202020-05-17T23:13:44 <shesek> why does `gettransaction` only list the fee for "send" transactions? is doing this for incoming transactions problematic for some reason, or just not implemented yet?
4212020-05-17T23:15:08 <sipa> yes, your wallet does not know the input value of non-wallet tx inputs
4222020-05-17T23:16:31 <shesek> I meant specifically for `gettransaction`, which is for wallet transactions only
4232020-05-17T23:16:44 <sipa> yes, but the inputs of a transaction that pays you are not yours
4242020-05-17T23:16:46 <shesek> bitcoin-qt does seem to somehow get it
4252020-05-17T23:16:51 <sipa> so your wallet does not know about them
4262020-05-17T23:17:54 <shesek> it does know them at validation time though, couldn't this get persisted alongside the other wallet tx information?
4272020-05-17T23:18:02 <sipa> it could :)
4282020-05-17T23:18:21 <sipa> but conceptually it doesn't really make sense; gettransaction shows the effect of a tx on your wallet
4292020-05-17T23:18:25 <sipa> you're not the one paying the fee
4302020-05-17T23:19:23 *** Dean_Guss has quit IRC
4312020-05-17T23:19:24 <sipa> but sure, it could be persisted
4322020-05-17T23:19:52 <shesek> its useful for incoming payments to know how likely they are to get confirmed. its something that a wallet gui should probably be displaying
4332020-05-17T23:20:00 <sipa> yeah, that makes sense
4342020-05-17T23:20:31 <shesek> does bitcoin-qt not show the fee of confirmed incoming transactions (ie where the spent prevouts are no longer in the utxo set)?
4352020-05-17T23:20:55 <shesek> I haven't used the gui in ages >_<
4362020-05-17T23:22:24 <shesek> how about including the "fee" in the `{get,list}transaction` rpcs for unconfirmed transactions only? the fee is really only useful for them, and it could be deducted by a pruning node
4372020-05-17T23:27:06 <sipa> i think it's cleaner to persist the fee in the wallet
4382020-05-17T23:27:15 <sipa> otherwise the wallet needs access to the utxo set
4392020-05-17T23:27:48 *** AaronvanW has quit IRC
4402020-05-17T23:28:17 <shesek> how does this work for outgoing transactions? persisted in the wallet?
4412020-05-17T23:29:03 *** bitdex has quit IRC
4422020-05-17T23:30:00 *** bitdex has joined #bitcoin-core-dev
4432020-05-17T23:33:12 <sipa> for outgoing transactions (at least when they're not coinjoins), you have the inputs anyway
4442020-05-17T23:35:41 <shesek> ah, so it wouldn't report a fee if any of the inputs are not from the wallet? this should probably get documented
4452020-05-17T23:36:24 <shesek> maybe something like "This is negative and only available for the 'send' category of transactions when all inputs are owned by the wallet."
4462020-05-17T23:42:41 <sipa> if it's persisted in the wallet it would work for every transaction
4472020-05-17T23:47:05 *** proofofkeags has joined #bitcoin-core-dev
4482020-05-17T23:56:27 <shesek> well, sign me up as someone eagerly waiting for this :)
4492020-05-17T23:57:19 <sipa> the best way to make it happen is to implement it :)
4502020-05-17T23:57:30 *** proofofkeags has quit IRC
4512020-05-17T23:57:31 *** Chris_Stewart_5 has quit IRC
4522020-05-17T23:58:04 *** proofofkeags has joined #bitcoin-core-dev