19:01:22 <laanwj> #startmeeting 
19:01:23 <core-meetingbot`> Meeting started Thu Jan 20 19:01:22 2022 UTC.  The chair is laanwj. Information about MeetBot at https://bitcoin.jonasschnelli.ch/ircmeetings.
19:01:23 <core-meetingbot`> Available commands: action commands idea info link nick
19:01:29 <sipsorcery> hi
19:01:30 <b10c> hi
19:01:40 <laanwj> #bitcoin -core-dev Meeting: achow101 _aj_ amiti ariard BlueMatt cfields Chris_Stewart_5 darosior digi_james dongcarl elichai2 emilengler fanquake fjahr gleb glozow gmaxwell gwillen hebasto instagibbs jamesob jarolrod jb55 jeremyrubin jl2012 jnewbery jonasschnelli jonatack jtimon kallewoof kanzure kvaciral laanwj larryruane lightlike luke-jr maaku marcofalke meshcollider michagogo moneyball
19:01:42 <laanwj> morcos nehan NicolasDorier paveljanik petertodd phantomcircuit promag provoostenator ryanofsky sdaftuar sipa vasild
19:01:44 <hebasto> hi
19:01:48 <jonatack> hi
19:01:52 <MarcoFalke> hi
19:01:55 <achow101> hi
19:01:58 <_aj_> hi
19:01:59 <lightlike> hi
19:02:01 <kvaciral[m]> hi
19:02:35 <laanwj> hi
19:02:46 <laanwj> it looks like there have been no proposed meeting topics this week (this can be done using #proposedmeetingtopic <topic>), any last-minute ones?
19:02:55 <jarolrod> Hi
19:03:32 <jeremyrubin> hi
19:03:35 <sipa> hi
19:04:39 <laanwj> #topic High priority for review
19:04:39 <core-meetingbot`> topic: High priority for review
19:05:06 <MarcoFalke> Can I have #23438 pls
19:05:07 <laanwj> https://github.com/bitcoin/bitcoin/projects/8  : 9 blockers, 1 chasing concept ACK
19:05:09 <gribble> https://github.com/bitcoin/bitcoin/issues/23438 | refactor: Use spans of std::byte in serialize by MarcoFalke · Pull Request #23438 · bitcoin/bitcoin · GitHub
19:06:06 <laanwj> MarcoFalke: added
19:06:17 <MarcoFalke> thx
19:06:51 <cfields> hi
19:08:01 <laanwj> anything else to add/remove, or is anything ready for merge?
19:09:11 <_aj_> i think #23508 is rfm but it needs a few more acks :)
19:09:13 <gribble> https://github.com/bitcoin/bitcoin/issues/23508 | Add getdeploymentinfo RPC by ajtowns · Pull Request #23508 · bitcoin/bitcoin · GitHub
19:09:14 <jonatack> #23604 had 4 ACKs before the last rebase, should be close
19:09:16 <gribble> https://github.com/bitcoin/bitcoin/issues/23604 | Use Sock in CNode by vasild · Pull Request #23604 · bitcoin/bitcoin · GitHub
19:09:43 <laanwj> ah yes #23604 should definitely be close, and #23508 too
19:09:45 <gribble> https://github.com/bitcoin/bitcoin/issues/23604 | Use Sock in CNode by vasild · Pull Request #23604 · bitcoin/bitcoin · GitHub
19:09:46 <gribble> https://github.com/bitcoin/bitcoin/issues/23508 | Add getdeploymentinfo RPC by ajtowns · Pull Request #23508 · bitcoin/bitcoin · GitHub
19:10:45 <jonatack> #22932 had acks by hebasto, achow101 and vasild. I updated it to take review feedback, re-acks welcome
19:10:47 <gribble> https://github.com/bitcoin/bitcoin/issues/22932 | Add CBlockIndex lock annotations, guard nStatus/nFile/nDataPos/nUndoPos by cs_main by jonatack · Pull Request #22932 · bitcoin/bitcoin · GitHub
19:11:00 <fjahr> hi
19:11:10 <jonatack> it now contains a few commits by hebasto and vasild
19:11:46 <jonatack> will look at #23508
19:11:48 <gribble> https://github.com/bitcoin/bitcoin/issues/23508 | Add getdeploymentinfo RPC by ajtowns · Pull Request #23508 · bitcoin/bitcoin · GitHub
19:11:49 <laanwj> great!
19:12:44 <fjahr> already started reviewing #23508, should be done soon as well
19:12:46 <gribble> https://github.com/bitcoin/bitcoin/issues/23508 | Add getdeploymentinfo RPC by ajtowns · Pull Request #23508 · bitcoin/bitcoin · GitHub
19:14:22 <laanwj> any other topics?
19:15:16 <_aj_> is the 23.0 milestone up to date?
19:15:54 <jeremyrubin> if there's nothing else i think it's interesting to talk about making a style-change epoch after split-off
19:16:08 <laanwj> _aj_: i don't know, do you see anything off?
19:16:25 <laanwj> style change epoch? oh no...
19:16:32 <_aj_> laanwj: just wonering if i should go through that a few times before feature freeze
19:16:34 <MarcoFalke> Yeah, let us know if anything is missing from it or should be removed
19:17:00 <MarcoFalke> https://github.com/bitcoin/bitcoin/milestone/52
19:19:28 <laanwj> #topic Style change after split-off (jeremyrubin)
19:19:29 <core-meetingbot`> topic: Style change after split-off (jeremyrubin)
19:19:34 <jeremyrubin> see #22969
19:19:35 <gribble> https://github.com/bitcoin/bitcoin/issues/22969 | Release schedule for 23.0 · Issue #22969 · bitcoin/bitcoin · GitHub
19:19:57 <jeremyrubin> the concept would be that after split off for T days bigger cleanups can be prioritized
19:20:08 <jeremyrubin> and then after T days, cleanups would not really be considered
19:20:15 <laanwj> not a fan of all-over-the-place changes that break every other PR just for style's sake tbh
19:20:17 <sipa> concept nack in general on invasive style changes
19:20:21 <laanwj> yeah
19:20:22 <jeremyrubin> this is to allow for cleanups while minimizing rebase work
19:20:31 <sipa> refactors that are useful for other reasons... sure, on a case-by-case basis
19:20:48 <laanwj> not sure anyone cares anymore but we used to not do this and i think for good reason
19:20:48 <jeremyrubin> i am nack on the invasive changes as well, but it seems people do continually want things like changing all C style casts
19:21:01 <laanwj> nack on the casts change, not only is it invasive it's risky
19:21:21 <_aj_> https://github.com/bitcoin/bitcoin/issues/15465 maybe relevant history/context
19:21:22 <jeremyrubin> or things like moving class A from file X to file Y to clean a linker supression
19:22:18 <_aj_> personally i like/prefer the "do useful work, and clean up as you go" approach that we're already doing
19:22:19 <laanwj> you mean breaking circular dependencies? yeah, that can be valid
19:22:25 <MarcoFalke> [20:20] <sipa> refactors that are useful for other reasons... sure, on a case-by-case basis
19:22:33 <jeremyrubin> e.g. #17786
19:22:34 <MarcoFalke> Agree with sipa that this is a case-by-case decision
19:22:34 <gribble> https://github.com/bitcoin/bitcoin/issues/17786 | refactor: Nuke policy/fees->mempool circular dependencies by hebasto · Pull Request #17786 · bitcoin/bitcoin · GitHub
19:22:38 <sipa> aj: agreew
19:22:59 <laanwj> i don't see that as much of a style thing
19:23:02 <jeremyrubin> well the point would more be to still do these things when they make sense, but contain it to a minimally disruptive part of the release cycle
19:23:09 <laanwj> circular dependencies are just bad design
19:23:22 <jeremyrubin> laanwj: but you can see how that's very disruptive for rebasing
19:23:28 <laanwj> sure
19:23:49 <jeremyrubin> so the point of a cleanup epoch would be to do cleanups like that only in e.g. the first month after split off
19:23:58 <_aj_> it's maximally disruptive for anything we want to backport to the latest release though?
19:24:04 <MarcoFalke> jeremyrubin: Why would a specific epoch be minimally disruptive? I'd say it depends on the case
19:24:09 <sipa> I conceptually agree that there are better/worse times to make invasive changes (assuming they're justified)... but they also have to be ready at that point.
19:24:18 <jeremyrubin> yeah i'm not tied to a specific time
19:24:24 <sipa> And indeed, when that is depends on the case.
19:24:38 <MarcoFalke> If you are touching a part of the code that no one else is touching, then the time doesn't matter
19:24:39 <jeremyrubin> just thinking that it might be something that leads to less contributor frustration
19:25:05 <jeremyrubin> if people who work on these things know when it is appropriate and people who don't don't have to argue for not merging a conflict
19:25:21 <MarcoFalke> If you are touding a part of the code that is touched by others, then you'll have to order the changes anyway
19:25:51 <sipa> Right, but the risk is that we designate a specific timeframe for refactors... and then we get a torrent of refactors in that time that all conflict with each other.
19:26:50 <jeremyrubin> possibly. although it's not clear it's a bigger problem than as is v.s. just more apparent
19:27:10 <jeremyrubin> if we queue up the refactors and people discuss them before investing time maybe that can help mitigate it
19:27:39 <MarcoFalke> I'd also expect it will be harder to find reviewers if there is a time pressure/specific time slot
19:28:53 <jonatack> seems best to remain ad hoc about it (as it is currently)
19:29:31 <MarcoFalke> If you are worries about invensting time, you can also ask during the meeting if the concept makes sense (or in an issue)
19:29:48 <jeremyrubin> i guess so, it seems sad though because i think it does contribute to burn out and frustration.
19:30:02 <jeremyrubin> i dont spend time on refactors like this anymore afaik so not really concerned for me
19:30:06 <MarcoFalke> Generally writing code is trivial compared to reading and reviewing not so
19:30:12 <MarcoFalke> s/not so//g
19:30:59 <jeremyrubin> i think the main benefit is for people not working on these
19:31:27 <jeremyrubin> If i have a feature/improvement X and there is also a refactor Y in the pipe that would confilict does it makes sense for me to refactor/rebase X?
19:31:48 <jeremyrubin> If I knew that the cleanup work would not pre-empt, i might spend time more actively on X rather than speculating on Y
19:31:53 <jonatack> one can make the argument that adding process can contribute to dev frustration as well
19:32:49 <jeremyrubin> jonatack: good point, process ==bureaucracy, however, it also leads to getting a clear answer on when or how something can proceed
19:33:12 <sipa> I think most frustration is simply due to lack of review feedback. I don't think that designating times for certain kinds of changes will improve that (and it may even work the other way around... "I was told this was the time for this kind of change, and I *still* don't get feedback!?")
19:33:12 <jeremyrubin> i personally favor clear answers v.s. unclear ones
19:33:12 <MarcoFalke> If you think that X should go before Y (or the other way round), you should say so in a review comment
19:33:46 <_aj_> drahtbot already says that when it reports conflicting prs
19:34:09 <jeremyrubin> _aj_ afaict drahbot doesn't analyze for merge order, just for conflicts
19:34:17 <jeremyrubin> and it's buggy w.r.t. silent conflicts
19:34:19 <_aj_> jeremyrubin: no it tells you to figure out which one should go first
19:34:31 <jeremyrubin> (err unsolvable)
19:34:31 <MarcoFalke> Jup, it was the intention behind the comment to focus reviewers on one of the conflicts
19:35:02 <MarcoFalke> DrahtBot re-runs all CI every 9 days, so (some) silent conflicts are now detected
19:35:17 <laanwj> that's good!
19:35:22 <jeremyrubin> anyhow it sounds like most people are happy with the status quo on this stuff so it's not a hill i'd die on
19:35:29 <MarcoFalke> DrahtBot can't tell you which order to merge. This is up to the reviewers
19:36:15 <sipa> I wouldn't say I'm happy - the frustration you speak of, and the impact on contributors is certainly real. I however don't think your suggestion will improve it.
19:36:28 <MarcoFalke> I mean frustration is certainly real, but I think a "refactor window" is not a clear and trivial to solve that.
19:36:29 <lightlike> seems in practice it's more up to the maintainers - I rarely see discussions about merge order amongst reviewers
19:36:57 <laanwj> it would be an indication to the maintainers
19:37:03 <MarcoFalke> lightlike: It is implicit in where people put their ACKs
19:37:14 <jeremyrubin> sipa: fair, i'm not tied to any specific suggestion. is there something else you think might be an improvement?
19:37:33 <sipa> I'm afraid I don't have concrete suggestions.
19:37:44 <laanwj> it definitely happens that someone asks for something to be merged before/after another PR
19:38:03 <jeremyrubin> is this a harmful thing to try over e.g. 2 releases?
19:38:05 <laanwj> not often, but it's also not often necessary
19:38:25 <jeremyrubin> or do we feel confident the problems would exceed the benefit?
19:39:15 <jeremyrubin> i'm all for trying something if the status quo isn't happy and if it doesnt help trying something else
19:39:26 <laanwj> that kind of coordination tends to be pretty hard to do in this project tbh
19:40:09 <sipa> I think it's such a case-by-case thing that trying to institute a process for it isn't helpful.
19:40:23 <MarcoFalke> Jup, would be hard to announce that the next 12 days from today are the "refactor window" and explain what that even means in practice
19:41:42 <_aj_> are there any refactoring tools that make it easy to see "oh you're just changing from Foo* x and x->bar to Foo& x and x.bar" these days?
19:42:02 <jeremyrubin> scripted-diff?
19:42:04 <MarcoFalke> "--word-diff-regex=." ?
19:42:24 <_aj_> i mean syntax aware, rather than textual
19:42:26 <jeremyrubin> there's no tooling for AFAIU testing a scripted diff against all current reported conflicting branches
19:43:32 <MarcoFalke> _aj_: You can write one based on https://github.com/llvm-mirror/clang/blob/master/tools/clang-diff/ClangDiff.cpp ;)
19:43:34 <_aj_> syntax aware can see I'm changing Foo::bar to Foo::m_bar and see that Baz::bar is unrelated while scripted-diff generally can't eg
19:44:07 <laanwj> syntax/structure aware refactoring tools for c++ are really difficult to do, people have been trying for decades at least
19:44:42 <laanwj> i think clang has some tooling nowadays but it's eally fiddly
19:45:19 <MarcoFalke> Let's use clang-refactor in a scripted-diff *hides
19:45:20 <laanwj> for easier to parse languages like java it's quite common
19:45:30 <laanwj> lol
19:46:08 <laanwj> any other topics?
19:46:20 <_aj_> MarcoFalke: yes, exactly! thanks!
19:46:55 <laanwj> #endmeeting