--- Log opened Wed Jul 20 00:00:26 2022
00:00 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:799b:1288:ff0f:633e] has quit [Ping timeout: 276 seconds]
00:06 -!- brunoerg [~brunoerg@187.183.43.40] has joined #bitcoin-core-pr-reviews
00:20 -!- __gotcha [~Thunderbi@natx-145.kulnet.kuleuven.be] has joined #bitcoin-core-pr-reviews
01:05 -!- brunoerg [~brunoerg@187.183.43.40] has quit [Ping timeout: 255 seconds]
01:07 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:799b:1288:ff0f:633e] has joined #bitcoin-core-pr-reviews
01:11 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:799b:1288:ff0f:633e] has quit [Ping timeout: 240 seconds]
01:13 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:799b:1288:ff0f:633e] has joined #bitcoin-core-pr-reviews
01:36 -!- Cory [~Cory@user/pasha] has joined #bitcoin-core-pr-reviews
01:53 -!- darosior6 [~darosior@194.36.189.246] has joined #bitcoin-core-pr-reviews
01:53 -!- lowhope [~lowhope@2602:fffa:fff:108a:0:16:3e86:c70e] has quit [Ping timeout: 240 seconds]
01:54 -!- koolazer [~koo@user/koolazer] has quit [Ping timeout: 256 seconds]
01:54 -!- darosior [~darosior@194.36.189.246] has quit [Ping timeout: 256 seconds]
01:54 -!- darosior6 is now known as darosior
01:56 -!- koolazer [~koo@user/koolazer] has joined #bitcoin-core-pr-reviews
01:56 -!- lowhope [~lowhope@2602:fffa:fff:108a:0:16:3e86:c70e] has joined #bitcoin-core-pr-reviews
02:21 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:799b:1288:ff0f:633e] has quit [Ping timeout: 268 seconds]
02:27 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:799b:1288:ff0f:633e] has joined #bitcoin-core-pr-reviews
03:31 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:799b:1288:ff0f:633e] has quit [Ping timeout: 240 seconds]
03:40 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:799b:1288:ff0f:633e] has joined #bitcoin-core-pr-reviews
04:43 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:799b:1288:ff0f:633e] has quit [Ping timeout: 255 seconds]
04:44 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:799b:1288:ff0f:633e] has joined #bitcoin-core-pr-reviews
06:50 -!- kouloumos [uid539228@id-539228.tinside.irccloud.com] has joined #bitcoin-core-pr-reviews
07:30 -!- adam2k [~adam2k@ip24-254-208-245.hr.hr.cox.net] has joined #bitcoin-core-pr-reviews
08:10 -!- __gotcha [~Thunderbi@natx-145.kulnet.kuleuven.be] has quit [Ping timeout: 240 seconds]
08:47 -!- Kaizen_Kintsugi_ [~Kaizen_Ki@bras-base-notlon0902w-grc-22-70-49-167-252.dsl.bell.ca] has joined #bitcoin-core-pr-reviews
08:48 -!- ellipizle [~ellipizle@105.112.225.13] has joined #bitcoin-core-pr-reviews
08:54 -!- ellipizle [~ellipizle@105.112.225.13] has quit [Quit: Connection closed]
08:58 -!- MacroFake [~none@72.5.34.65] has joined #bitcoin-core-pr-reviews
09:01 -!- Kaizen_Kintsugi_ [~Kaizen_Ki@bras-base-notlon0902w-grc-22-70-49-167-252.dsl.bell.ca] has quit [Remote host closed the connection]
09:04 -!- Kaizen_Kintsugi_ [~Kaizen_Ki@bras-base-notlon0902w-grc-22-70-49-167-252.dsl.bell.ca] has joined #bitcoin-core-pr-reviews
09:09 -!- Kaizen_Kintsugi_ [~Kaizen_Ki@bras-base-notlon0902w-grc-22-70-49-167-252.dsl.bell.ca] has quit [Remote host closed the connection]
09:20 -!- __gotcha [~Thunderbi@94.105.117.26.dyn.edpnet.net] has joined #bitcoin-core-pr-reviews
09:21 -!- adam2k [~adam2k@ip24-254-208-245.hr.hr.cox.net] has quit [Ping timeout: 268 seconds]
09:38 -!- Kaizen_Kintsugi_ [~Kaizen_Ki@70.49.167.252] has joined #bitcoin-core-pr-reviews
09:40 -!- dariusp [~dariusp@24.130.26.7] has joined #bitcoin-core-pr-reviews
09:41 -!- furszy [~furszy@user/furszy] has joined #bitcoin-core-pr-reviews
09:51 -!- Paul_C [~Paul_C@mobile-107-77-202-26.mobile.att.net] has joined #bitcoin-core-pr-reviews
09:53 -!- gamliel [~gamliel@190.87.162.97] has joined #bitcoin-core-pr-reviews
09:53 -!- cec [~cec@096-042-225-003.res.spectrum.com] has joined #bitcoin-core-pr-reviews
09:56 -!- wieland7 [~wieland@178.237.235.157] has joined #bitcoin-core-pr-reviews
09:59 -!- adam2k [~adam2k@ip24-254-208-245.hr.hr.cox.net] has joined #bitcoin-core-pr-reviews
09:59 -!- satsie [~satsie@c-24-218-105-103.hsd1.ct.comcast.net] has joined #bitcoin-core-pr-reviews
09:59 -!- Paul_C [~Paul_C@mobile-107-77-202-26.mobile.att.net] has quit [Quit: Connection closed]
10:00 < stickies-v> #startmeeting
10:00 -!- sanic [~sanic@186-78-232-183.baf.movistar.cl] has joined #bitcoin-core-pr-reviews
10:00 < larryruane> hi
10:00 < gamliel> hi
10:00 < furszy> hi
10:00 -!- Paul_C [~Paul_C@mobile-107-77-202-26.mobile.att.net] has joined #bitcoin-core-pr-reviews
10:00 < dariusp> hi
10:01 -!- Lov3r_Of_Bitcoin [~Lov3r_Of_@45-27-31-99.lightspeed.sntcca.sbcglobal.net] has joined #bitcoin-core-pr-reviews
10:01 < josie[m]> hi
10:01 < wieland7> hi
10:01 < satsie> hi
10:01 < Paul_C> Hey everyone
10:01 < Lov3r_Of_Bitcoin> hello
10:01 < adam2k> 👋
10:01 < stickies-v> welcome everyone! Today we're looking at #25218, authored by furszy. The notes and questions are available on https://bitcoincore.reviews/25218
10:02 < michaelfolkson2> hi
10:02 < stickies-v> very glad to see that we've got the author himself present here as well!
10:02 -!- nassersaazi [~nassersaa@h12b0.n1.ips.mtn.co.ug] has joined #bitcoin-core-pr-reviews
10:02 < stickies-v> anyone joining us for the first time today? even if you're just lurking, feel free to say hi!
10:02 < gamliel> hi  o/
10:02 -!- michaelfolkson2 is now known as michaelfolkson
10:02 < adam2k> First time for me
10:02 -!- khorner [~khorner@rrcs-71-42-185-2.sw.biz.rr.com] has joined #bitcoin-core-pr-reviews
10:03 < gamliel> just lurking, eager to collab some day :)
10:03 < stickies-v> glad you found your way here adam2k, welcome and don't hesitate to ask or participate if you feel like it
10:03 -!- Paul_C [~Paul_C@mobile-107-77-202-26.mobile.att.net] has quit [Client Quit]
10:03 < stickies-v> yes - lurkers very welcome!
10:03 < stickies-v> who got the chance to review the PR or read the notes? (y/n)
10:03 < gamliel> <3
10:04 < adam2k> y
10:04 < khorner> lurker - n
10:04 < satsie> y
10:04 < wieland7> partially
10:04 < gamliel> y
10:04 < stickies-v> nice, lots of eyes on the code!
10:04 < dariusp> notes - y, PR - briefly
10:04 < stickies-v> for those of you who were able to review, would you give it a Concept ACK, Approach ACK, Tested ACK, or NACK?
10:05 < josie[m]> y (read notes and ended up needing to rebase on top of this PR) 
10:05 < stickies-v> (and in general, even though the PR is already merged, post-merge (N)ACKs are always very welcome too)
10:05 -!- nassersaazi [~nassersaa@h12b0.n1.ips.mtn.co.ug] has quit [Client Quit]
10:06 < josie[m]> Aproach ACK
10:06 < stickies-v> josie[m]: good, more usage :-D
10:06 -!- sanic [~sanic@186-78-232-183.baf.movistar.cl] has quit [Quit: Connection closed]
10:06 < Lov3r_Of_Bitcoin> Concept ACK
10:06 < satsie> approach ack - but I was really surprised to see a bunch of things I hadn't considered in the PR discussion
10:06 < gamliel> sorry, my first time in this meeting too :P
10:06 < schmidty_> hi
10:06 -!- nassersaazi [~nassersaa@h12b0.n1.ips.mtn.co.ug] has joined #bitcoin-core-pr-reviews
10:07 < stickies-v> satsie: and there's even more discussion in other places too, but we'll get to that in a second! but the discussion is always a nice place to see how different people catch different things and why having many eyes on the code is so important
10:08 < stickies-v> very welcome, gamliel!
10:08 < satsie> indeed!
10:08 < adam2k> Approach ACK
10:08 < stickies-v> alright let's get going with the first question
10:08 < stickies-v> What are the different types of parameters and return values that the PR assumes we commonly use in a function signature? Which of these parameters are affected by the PR?
10:09 -!- sanya [~sanya@109-93-178-218.dynamic.isp.telekom.rs] has joined #bitcoin-core-pr-reviews
10:10 < adam2k> bilingual_str was removed from the function signature in a bunch of places.
10:10 < satsie> I think the PR assumes most function signatures have your typical set of in parameters, and two out parameters (error and return value). The PR affects the two out parameters
10:11 < larryruane> satsie: yes, and typically the return value type is `bol`
10:11 < larryruane> *bool
10:11 < larryruane> (indicating success or failure)
10:11 < satsie> ah, yes! good point
10:11 < stickies-v> satsie & larryruane - yes exactly, those I'd say are the main 4 categories here
10:12 < stickies-v> so this PR should not really affect the input parameters, but it does affect the function signature for all 3 other categories
10:13 < stickies-v> Why does `BResult` have a separate constructor `BResult(const bilingual_str& error)` that seems to do the exact same as the templated constructor `BResult(const T& _obj)`? Does this introduce any edge cases or limitations, and if so - are they documented?
10:13 < satsie> stickies-v can you clarify what the 3 other categories are?
10:13 < stickies-v> (note: the discussion is async, so even if i move on to the next question, feel free to continue the discussion on previous points)
10:14 < larryruane> yeah, I couldn't figure this out! I commented out the bilingual_str constructor, and it still compiled
10:14 < stickies-v> satsie: the ones you mentioned, actually. 1) inputs 2) outputs 3) error handling
10:15 < stickies-v> error handling is also a kind of output of course, but I think it's still different enough
10:15 < satsie> 👍
10:16 < larryruane> is the separate constructor just for code clarity? so it's understood that there are these two different types of `BResult`s? I'm unsure
10:16 < satsie> for the second question, I was also a little stumped. I'm sure there's some C++ stuff going on here that I don't know about but the two constructors seem to enforce that m_variant can only be  bilingual_str& OR  T&
10:16 < adam2k> For the second question does the separate constructor exist to be an overloaded constructor for this particular type?
10:16 < satsie> +1 to what you're saying about code clarity Larry
10:17 < stickies-v> larryruane: I'm very surprised it would still compile. If e.g. a function is meant to return `int`, we'd define the return type as `BResult<int>`. If we then have an error and just return "some error string", then that shouldn't compile, I think?
10:18 < stickies-v> (without the dedicated `bilingual_str&` constructor)
10:18 < larryruane> maybe I messed up :)
10:19 < furszy> yeah, that shouldn't be compiling.
10:19 < satsie> is it to show that you can create a BResult with just one input, and the other part of the result (the error or the T) is inferred?
10:19 < furszy> should get something like "no known conversion from 'bilingual_str' to 'const BResul<something>"
10:19 -!- SOy_yo [~SOy_yo@172.92.166.62] has joined #bitcoin-core-pr-reviews
10:20 < larryruane> i checked again and dont see anything i did wrong ... i'm using clang BTW (tho shouldn't matter)
10:20 < josie[m]> adam2k: what do you mean by "overloaded constructor"?
10:21 -!- svav [~svav@82-69-86-143.dsl.in-addr.zen.co.uk] has joined #bitcoin-core-pr-reviews
10:21 < stickies-v> satsie adam2k: so `BResult` is templated to whatever we expect a function to return, e.g. an `int` or any other type. But we also want it to be straightforward (and obvious) to raise an error within the function. So by overloading the constructor with another `bilingual_str&` constructor, to raise an error anywhere in the function we can just return a (bilingual) string
10:22 < furszy> larryruane: just checked it locally and build failed.
10:22 < larryruane> why isn't the error type templated? Is it just because `bilingual_str` is so common, we don't think anything else will be needed?
10:23 < larryruane> furszy: ok I'll investigate later, thanks
10:23 < adam2k> josibake___ just that the parameters are different for the constructor in lines 21-23 here https://github.com/bitcoin/bitcoin/pull/25218/files#diff-dd552c1ad61f5e2027fcef75f3a0ba027d69b5617931b3574e5d6ef2d3cbebe5R21-R23
10:23 < stickies-v> larryruane: yes I believe that was the consensus in the discussion but I think furszy may be able to elaborate
10:24 < josie[m]> adam2k: ah! got it 
10:24 < stickies-v> removing the `bilingual_str&` constructor also fails to compile for me here
10:25 < satsie> larryruane: Here's a link to a comment on an argument to not make `bilingual_str` customizable: https://github.com/bitcoin/bitcoin/pull/25218#issuecomment-1161843649
10:25 < josie[m]> larryruane, furszy: i also had the same question regarding error type templates 
10:25 < furszy> stickies-v: yeah ok, the goal of this initial implementation was to introduce the BResult class without the generic error. Just the simplest, and more beneficial, use case for it.
10:26 < wieland7> also get a compiler error when commenting out the constructor
10:26 < larryruane> satsie: thanks!
10:26 < adam2k> stickies-v I'm still confused on the previous comment about `bilingual_str&`.  Maybe I'm just rusty on C++, but how does the `BResult(const T& _obj)` constructor differ from `BResult(const bilingual_str& error)`?
10:27 < adam2k> both look like pass by reference objects to me, but one has the specific `bilingual_str` type, right?
10:27 < stickies-v> adam2k: look at commit https://github.com/bitcoin/bitcoin/pull/25218/commits/111ea3ab711414236f8678566a7884d48619b2d8 , for example
10:28 -!- Kaizen_Kintsugi_ [~Kaizen_Ki@70.49.167.252] has quit [Remote host closed the connection]
10:28 < stickies-v> you'll see that the return type of `getNewDestination` becomes `virtual BResult<CTxDestination> `
10:29 < stickies-v> this means that the templated constructor now expects `T` to be of type `CTxDestination`
10:30 < stickies-v> just because something is templated, doesn't mean that just accepts any type. Even though templates can infer types automatically, in this case we always explicitly specify the type in our function signature
10:30 -!- BlueMoon [~BlueMoon@187.193.213.97] has joined #bitcoin-core-pr-reviews
10:31 < adam2k> ah!  Thanks, got it.
10:31 < furszy> josie[m]: initially, I implemented it a bit different. There was a pure generic base class Result<T, E> and a specialization of it with Result<T, bilingual_str>. But.. not many were happy introducing a class that wasn't connected to any function yet. So, ended up unifying them.
10:32 < stickies-v> there was a second part to the question though that I think hasn't been answered yet
10:32 < larryruane> yeah what helps me understand templating is to remember that it generates separate code for each type, like actually separate, different places in memory!
10:32 < stickies-v> "Does this introduce any edge cases or limitations, and if so - are they documented?"
10:32 -!- khorner [~khorner@rrcs-71-42-185-2.sw.biz.rr.com] has quit [Quit: Connection closed]
10:32 < josie[m]> furszy: thanks! catching up on the review comments and it makes more sense 
10:32 < stickies-v> larryruane: yeah thanks that's a helpful way to think about it!
10:33 < stickies-v> hint: what if we have a function that produces a `bilingual_str` as an output?
10:34 -!- SOy_yo [~SOy_yo@172.92.166.62] has quit [Quit: Connection closed]
10:34 < larryruane> stickies-v: you mean an existing function (with that return type), and we want to change it to a `BResult` type?
10:35 < satsie> A function that produces a `bilingual_str` as an output without intending for it to be an error?
10:35 < adam2k> Does that mean that the destination could be an error?
10:35 < adam2k> +1 to @sat
10:35 < adam2k> +1 to satsie
10:35 -!- Paul_C [~Paul_C@mobile-107-77-202-26.mobile.att.net] has joined #bitcoin-core-pr-reviews
10:36 < larryruane> oh, you probably can't do `BResult<bilingual_str>`
10:36 < stickies-v> yeah bilingual_str isn't exclusively used for errors, I'm not expert in the GUI but I believe that's where it's mostly used to represent translations
10:38 < larryruane> yes i see the problem there ... maybe there should have been a new error string type?
10:38 < adam2k> Or different error handling for this case?
10:39 < larryruane> (could be just a wrapper around a bilingual_str)
10:39 -!- Paul_C [~Paul_C@mobile-107-77-202-26.mobile.att.net] has quit [Client Quit]
10:39 < stickies-v> there are 2 follow up PRs that tackle this: https://github.com/bitcoin/bitcoin/pull/25608 and https://github.com/bitcoin/bitcoin/pull/25601
10:39 < satsie> got it. So to take it a step further, `BResult.HasRes()` assumes that the presence of a bilingual_str means there is an error, and in cases when bilingual_str doesn't actually mean an error, the caller is going to run into trouble
10:40 < stickies-v> satsie: exactly! it was a bit of a trick question, because it's not necessarily the constructor that's problematic, it's the HasRes() function
10:41 < stickies-v> Do you know of any other commonly used return type(s) in the codebase that are similar to `BResult`?
10:42 < satsie> woo hoo! so that follow up PR you just posted is an extension of the discussion in the original PR about the choice to make `bilingual_str` customizable/generic, right? (which furszy commented on a bit earlier in this chat)
10:42 < larryruane> oh this is interesting, i also get the compile error with gcc, but not with clang! maybe someone else can try clang
10:42 -!- Paul_C [~Paul_C@pool-74-96-218-208.washdc.fios.verizon.net] has joined #bitcoin-core-pr-reviews
10:43 < larryruane> (this is if i've commented out `BResult(const bilingual_str& error) : m_variant(error) {}` in result.h)
10:43 < stickies-v> satsie: yeah, and a couple more extensions too (which I'll be getting to in this very question actually!)
10:45 < adam2k> RPCResult looks like it might be another return type that is similar to BResult?
10:46 < josie[m]> skimming these follow-up PRs and both are really informative. any interest in adding them to a follow-up PR review club? 
10:48 < stickies-v> adam2k: interesting, I hadn't thought of that. RPCResult does standardize return types for the RPC, but maybe not really with that much of a focus on error handling
10:48 < larryruane> stickies-v: a similar return type, or at least has the same general goal, i would say is `std::optional<T>`
10:49 < stickies-v> larryruane: yes exactly, I think `std::optional` and `BResult` serve very similar purposes, where `BResult` adds support for accessing the error message
10:49 < larryruane> i see many places where an object is return inside the `optional` if successful, otherwise return `nullopt`
10:50 < stickies-v> and https://github.com/bitcoin/bitcoin/pull/25608 also aims to streamline this, by keeping the interface identical with `std::optional` (e.g. using `value()`, `value_or()`, `has_value()` functions as well as overloading the pointer `*` and `->` operators)
10:50 < larryruane> probably another similar practice is to return a pointer (a `std::unique_ptr`) where it's nullptr if error
10:50 < josie[m]> larryruane: ive been really confused by this in the past.. the use of nullopt and std::optional doesn't seem to be consistent 
10:51 < josie[m]> stickies-v, larryruane: could we eventually replace all the uses of std::optional with BResult (or something like it)?
10:51 -!- Paul_C [~Paul_C@pool-74-96-218-208.washdc.fios.verizon.net] has quit [Quit: Connection closed]
10:52 < stickies-v> josie[m]: what kind of inconsistency do you mean? that some functions use `std::unique_ptr` and others use `std::optional`?
10:52 < larryruane> stickies-v: I *think* so, are you getting now to question 6?
10:53 < stickies-v> hmm I'm not sure that would make sense. Not all functions require access to further error handling data, I think there are quite a few cases where getting a `std::nullopt` is perfectly clear and then using `std::optional` seems like a good choice?
10:53 < stickies-v> larryruane: hmm I mean it all kinda overlaps but I don't think so? :-D
10:54 < larryruane> stickies-v: +1 good point
10:54 < furszy> agree, BResult is useful when the function retrieves something else aside from the succeed value.
10:54 < josie[m]> stickies-v: yes, that, and also im thinking of another specific example but looking at it again, it might be unrelated 
10:55 < larryruane> furszy: when you say succeed value, do you mean the boolean?
10:55 < stickies-v> larryruane: OHH sorry I was looking at question 5 instead of 6. yes, you're right!
10:56 < furszy> larryruane: right now, BResult is implemented as an OR (internally uses an std::variant which is analogous to an union). It contains the succeed object OR the failure object (which is currently hardcoded to a single type)
10:56 < larryruane> furszy: +1 thanks
10:57 < stickies-v> let's wrap it up with a final discussion question
10:57 < stickies-v> Should someone now follow up with a big PR that refactors all functions that would benefit from using `BResult`? Why (not)?
10:57 < larryruane> no, it would be too disruptive, many unmerged PRs and downstream projects would need rebase, it's not currently broken, use for new code
10:57 < larryruane> but it is I think nice to initially use it in a few places, to make sure the interface is good and the code actually works! and also there can be further improvements!
10:58 < stickies-v> yeah I think you raise 2 very important points
10:58 < satsie> also it looks like there are already changes underway to upgrade BResult. It wouldn't make sense to do a sweeping refactor PR until some of that dust settles
10:58 < josie[m]> larryruane: ++1 
10:58 < larryruane> (i just learned about the further improvements here!)
10:58 < adam2k> yeah, I'd agree with larryruane.  It's probably better to make this a pattern that is implemented in future PRs.
10:58 < stickies-v> we're already kind of short on review capacity, so dumping a huge PR like that would not be very responsible
10:59 < stickies-v> and also that as we're using it in more and more places, we can iteratively improve/extend the interface
10:59 < josie[m]> also a really good point that even tho the first PR has been merged, folks are actively still iterating on the design 
10:59 < larryruane> yeah and you can't really do a scripted diff for this, unfortunately
11:00 < josie[m]> satsie: +1 
11:00 < stickies-v> i'd on the plus side say it would be very nice to have more uniform coding patterns across the codebase, makes it much easier for newcomes to onboard the codebase
11:00 < stickies-v> but, a very high price to pay
11:00 < adam2k> +1
11:01 < stickies-v> alright now that we're all in agreement, looks like a nice place to wrap it up!
11:01 < stickies-v> #endmeeting
11:01 < Lov3r_Of_Bitcoin> thank you!
11:01 -!- svav [~svav@82-69-86-143.dsl.in-addr.zen.co.uk] has quit [Quit: Connection closed]
11:01 < satsie> thanks stickies-v for hosting and thanks furszy for authoring the pr and attending!
11:01 < stickies-v> thank you furszy for authoring this very welcome improvement, and to everyone for preparing, attending and participating!
11:01 < josie[m]> thanks for hosting, stickies-v!
11:01 < adam2k> thanks all!  That was fun.
11:01 < larryruane> is this something that should be added to `developer-notes.md`? or just let people learn by example?
11:01 < dariusp> thank you stickies-v!
11:01 < larryruane> thanks stickies-v this was great, good hosting!
11:01 < furszy> thanks <3
11:01 < josie[m]> also thanks to furszy, really interesting stuff 
11:01 < stickies-v> this review club was meant to be a bit more actionable than usual. when contributing code in the future, it's quite likely you'll be able to benefit from `BResult` (or its successor(s) in the works), so hoping to see more elegant function signatures going forward!
11:02 < josie[m]> (altho annoying to rebase when i had no idea what Bresult was 😆)
11:02 < stickies-v> oh yeah larryruane I think that could be a good idea!
11:02 -!- Lov3r_Of_Bitcoin [~Lov3r_Of_@45-27-31-99.lightspeed.sntcca.sbcglobal.net] has quit [Quit: Connection closed]
11:03 < josie[m]> +1 for dev notes, but maybe after some of the followups have been addressed? 
11:03 -!- stickies-v [sid544753@id-544753.uxbridge.irccloud.com] has left #bitcoin-core-pr-reviews []
11:03 -!- stickies-v [sid544753@id-544753.uxbridge.irccloud.com] has joined #bitcoin-core-pr-reviews
11:04 -!- cec [~cec@096-042-225-003.res.spectrum.com] has quit [Quit: Connection closed]
11:04 < larryruane> a good thing for reviewers to do now would be to look for places in _unmerged_ PRs that we're reviewing, and suggesting to use `BResult` ... and appear wise as a tree full of owls!
11:04 < satsie> stickies-v wanted to dig into what you were mentioning about being short on review capacity. I'm totally new to core, but I know that one small line of code can have cascading effects, and that thorough review is needed for every proposed change. I see there are 300+ open PRs. Is that typical for bitcoin core? (i am prepared for you to say yes
11:04 < satsie> 😅)
11:05 < satsie> haha that's a good idea larryruane!
11:06 < furszy> larryruane: lol. I did that with josie's PR hehe.
11:06 < furszy> *on
11:06 < stickies-v> satsie: I haven't been around for long enough to answer that, sorry. I also didn't mean to sound alarming, more just that review time is a very scarce good
11:07 -!- sanya [~sanya@109-93-178-218.dynamic.isp.telekom.rs] has quit [Quit: Connection closed]
11:07 -!- nassersaazi [~nassersaa@h12b0.n1.ips.mtn.co.ug] has quit [Quit: Connection closed]
11:08 < satsie> oh! I didn't take it that way at all :)  I interpreted it as encouragement for anyone who wants to get more involved. Reviewing certainly sounds like a more attainable goal than opening PRs
11:08 < josie[m]> satsie: definitely agree that review tends to be a bottleneck in core, but i think looking at "open PRs" as a metric can be misleading
11:09 < satsie> oooo that's helpful to know 👍
11:09 < larryruane> This may be useful: https://github.com/bitcoin/bitcoin/projects/8
11:09 < josie[m]> for example, many open PRs are stalled or abandoned due to lack of interest from the author
11:10 < larryruane> but TBH I haven't used that list very much, because they tend to be very large PRs that I don't have the background for
11:11 < satsie> larryruane I haven't seen that project view before. That's a great list of the big "rocks" that are on a lot of people's radars
11:11 < josie[m]> id also say reviewing other people's PRs can be a great learning tool! ill often pick a PR that is interesting, spend some time learning about the relevant parts of the code + background info, and then finish my mini-lesson with a review. it's a nice way to have a tangible deliverable when you are still in the learning phase 
11:13 < satsie> I like that approach josie! especially about having that tangible deliverable
11:14 -!- sanya [~sanya@109-93-178-218.dynamic.isp.telekom.rs] has joined #bitcoin-core-pr-reviews
11:22 < gamliel> larryruane: that's interesting what you mentioned, about the background and the PR's size. Sorry for this newbie question: what's the very basic bg with cpp in order to someone who want to collaborate in PR review? or your comment was meant in the bg of the feature?
11:23 < gamliel> my first day here and just want to know an advice from the ones whom have some days here before me '=D
11:28 -!- satsie [~satsie@c-24-218-105-103.hsd1.ct.comcast.net] has quit [Quit: Connection closed]
11:32 -!- adam2k [~adam2k@ip24-254-208-245.hr.hr.cox.net] has quit [Ping timeout: 255 seconds]
11:43 -!- BlueMoon [~BlueMoon@187.193.213.97] has quit [Quit: Connection closed]
11:45 -!- gamliel [~gamliel@190.87.162.97] has quit [Quit: Connection closed]
11:45 -!- sanya [~sanya@109-93-178-218.dynamic.isp.telekom.rs] has quit [Quit: Connection closed]
11:50 -!- __gotcha [~Thunderbi@94.105.117.26.dyn.edpnet.net] has quit [Ping timeout: 268 seconds]
11:50 -!- Awoho [~Awoho@188.124.153.93] has joined #bitcoin-core-pr-reviews
11:59 -!- Awoho [~Awoho@188.124.153.93] has quit [Quit: Connection closed]
12:08 -!- __gotcha [~Thunderbi@94.105.117.26.dyn.edpnet.net] has joined #bitcoin-core-pr-reviews
12:16 -!- __gotcha [~Thunderbi@94.105.117.26.dyn.edpnet.net] has quit [Remote host closed the connection]
12:19 -!- __gotcha [~Thunderbi@94.105.117.26.dyn.edpnet.net] has joined #bitcoin-core-pr-reviews
12:52 -!- wieland7 [~wieland@178.237.235.157] has quit [Quit: Connection closed]
12:57 -!- dariusp [~dariusp@24.130.26.7] has quit [Ping timeout: 268 seconds]
13:47 -!- Kaizen_Kintsugi_ [~Kaizen_Ki@bras-base-notlon0902w-grc-22-70-49-167-252.dsl.bell.ca] has joined #bitcoin-core-pr-reviews
13:57 -!- Kaizen_Kintsugi_ [~Kaizen_Ki@bras-base-notlon0902w-grc-22-70-49-167-252.dsl.bell.ca] has quit [Remote host closed the connection]
14:05 -!- hotbeat [~hotbeat@64.145.93.205] has joined #bitcoin-core-pr-reviews
14:09 -!- afmencken [~afmencken@static-198-54-131-158.cust.tzulo.com] has joined #bitcoin-core-pr-reviews
15:13 -!- deadslug [~enno@2600:1010:b044:c772:dd00:61da:c511:49bb] has joined #bitcoin-core-pr-reviews
15:39 -!- kouloumos [uid539228@id-539228.tinside.irccloud.com] has quit [Quit: Connection closed for inactivity]
15:43 -!- __gotcha [~Thunderbi@94.105.117.26.dyn.edpnet.net] has quit [Ping timeout: 268 seconds]
16:25 -!- Kaizen_Kintsugi_ [~Kaizen_Ki@bras-base-notlon0902w-grc-22-70-49-167-252.dsl.bell.ca] has joined #bitcoin-core-pr-reviews
16:26 -!- Kaizen_Kintsugi_ [~Kaizen_Ki@bras-base-notlon0902w-grc-22-70-49-167-252.dsl.bell.ca] has quit [Remote host closed the connection]
16:42 -!- hotbeat [~hotbeat@64.145.93.205] has quit [Ping timeout: 240 seconds]
17:03 -!- Kaizen_Kintsugi_ [~Kaizen_Ki@bras-base-notlon0902w-grc-22-70-49-167-252.dsl.bell.ca] has joined #bitcoin-core-pr-reviews
17:09 -!- deadslug [~enno@2600:1010:b044:c772:dd00:61da:c511:49bb] has quit [Ping timeout: 244 seconds]
17:21 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:799b:1288:ff0f:633e] has quit [Remote host closed the connection]
17:22 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:799b:1288:ff0f:633e] has joined #bitcoin-core-pr-reviews
17:26 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:799b:1288:ff0f:633e] has quit [Ping timeout: 240 seconds]
17:30 -!- deadslug [~enno@157.sub-174-194-148.myvzw.com] has joined #bitcoin-core-pr-reviews
17:31 -!- deadslug [~enno@157.sub-174-194-148.myvzw.com] has quit [Client Quit]
17:37 -!- deadslug [~enno@2600:1010:b01e:be68:dd00:61da:c511:49bb] has joined #bitcoin-core-pr-reviews
17:43 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:799b:1288:ff0f:633e] has joined #bitcoin-core-pr-reviews
17:49 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:799b:1288:ff0f:633e] has quit [Ping timeout: 272 seconds]
18:06 -!- Kaizen_Kintsugi_ [~Kaizen_Ki@bras-base-notlon0902w-grc-22-70-49-167-252.dsl.bell.ca] has quit [Remote host closed the connection]
18:07 -!- Kaizen_Kintsugi_ [~Kaizen_Ki@bras-base-notlon0902w-grc-22-70-49-167-252.dsl.bell.ca] has joined #bitcoin-core-pr-reviews
18:20 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:799b:1288:ff0f:633e] has joined #bitcoin-core-pr-reviews
18:20 -!- sunfarmsbtc [~sunfarmsb@102.89.40.224] has joined #bitcoin-core-pr-reviews
18:30 -!- sunfarmsbtc [~sunfarmsb@102.89.40.224] has quit [Ping timeout: 272 seconds]
18:58 -!- Kaizen_Kintsugi_ [~Kaizen_Ki@bras-base-notlon0902w-grc-22-70-49-167-252.dsl.bell.ca] has quit [Remote host closed the connection]
18:59 -!- Kaizen_Kintsugi_ [~Kaizen_Ki@bras-base-notlon0902w-grc-22-70-49-167-252.dsl.bell.ca] has joined #bitcoin-core-pr-reviews
19:03 -!- Kaizen_Kintsugi_ [~Kaizen_Ki@bras-base-notlon0902w-grc-22-70-49-167-252.dsl.bell.ca] has quit [Ping timeout: 272 seconds]
19:17 -!- Kaizen_Kintsugi_ [~Kaizen_Ki@bras-base-notlon0902w-grc-22-70-49-167-252.dsl.bell.ca] has joined #bitcoin-core-pr-reviews
19:23 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:799b:1288:ff0f:633e] has quit [Ping timeout: 240 seconds]
19:32 -!- brunoerg [~brunoerg@187.183.43.40] has joined #bitcoin-core-pr-reviews
19:51 -!- Kaizen_K_ [~Kaizen_Ki@bras-base-notlon0902w-grc-22-70-49-167-18.dsl.bell.ca] has joined #bitcoin-core-pr-reviews
19:52 -!- Kaizen_Kintsugi_ [~Kaizen_Ki@bras-base-notlon0902w-grc-22-70-49-167-252.dsl.bell.ca] has quit [Ping timeout: 268 seconds]
19:56 -!- Kaizen_K_ [~Kaizen_Ki@bras-base-notlon0902w-grc-22-70-49-167-18.dsl.bell.ca] has quit [Ping timeout: 272 seconds]
20:11 -!- Kaizen_Kintsugi_ [~Kaizen_Ki@bras-base-notlon0902w-grc-22-70-49-167-18.dsl.bell.ca] has joined #bitcoin-core-pr-reviews
20:11 -!- greypw254600 [~greypw254@grey.pw] has joined #bitcoin-core-pr-reviews
20:34 -!- brunoerg [~brunoerg@187.183.43.40] has quit [Ping timeout: 268 seconds]
20:35 -!- Kaizen_Kintsugi_ [~Kaizen_Ki@bras-base-notlon0902w-grc-22-70-49-167-18.dsl.bell.ca] has quit [Ping timeout: 272 seconds]
20:43 -!- brunoerg [~brunoerg@187.183.43.40] has joined #bitcoin-core-pr-reviews
21:05 -!- david-bakin [~david-bak@c-174-61-163-5.hsd1.wa.comcast.net] has quit [Ping timeout: 240 seconds]
21:18 -!- deadslug [~enno@2600:1010:b01e:be68:dd00:61da:c511:49bb] has quit [Quit: Konversation terminated!]
21:46 -!- brunoerg [~brunoerg@187.183.43.40] has quit [Ping timeout: 272 seconds]
21:47 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:d16a:e8a7:2a08:2cd3] has joined #bitcoin-core-pr-reviews
22:36 -!- ghost43_ [~ghost43@gateway/tor-sasl/ghost43] has quit [Remote host closed the connection]
22:36 -!- ghost43 [~ghost43@gateway/tor-sasl/ghost43] has joined #bitcoin-core-pr-reviews
22:52 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:d16a:e8a7:2a08:2cd3] has quit [Ping timeout: 272 seconds]
22:58 -!- brunoerg [~brunoerg@187.183.43.40] has joined #bitcoin-core-pr-reviews
23:03 -!- brunoerg [~brunoerg@187.183.43.40] has quit [Ping timeout: 268 seconds]
23:04 -!- brunoerg [~brunoerg@187.183.43.40] has joined #bitcoin-core-pr-reviews
23:24 -!- zan [~zan@user/zan] has left #bitcoin-core-pr-reviews [Leaving]
--- Log closed Thu Jul 21 00:00:28 2022