19:02:12 <laanwj> #startmeeting 
19:02:12 <core-meetingbot> Meeting started Thu Oct 27 19:02:12 2022 UTC.  The chair is laanwj. Information about MeetBot at https://bitcoin.jonasschnelli.ch/ircmeetings.
19:02:12 <core-meetingbot> Available commands: action commands idea info link nick
19:02:21 <brunoerg> hi
19:02:24 <instagibbs> oh hi
19:02:29 <luke-jr> hi
19:02:30 <vasild> hi
19:02:32 <ariard> hi
19:02:35 <josie[m]> hi
19:02:35 <hebasto> hi
19:02:37 <laanwj> #bitcoin -core-dev Meeting: achow101 _aj_ amiti ariard b10c 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
19:02:39 <laanwj> moneyball morcos nehan NicolasDorier paveljanik petertodd phantomcircuit promag provoostenator ryanofsky sdaftuar sipa vasild
19:02:45 <achow101> hi
19:02:50 <lightlike> hi
19:02:53 <laanwj> welcome to the weekly general bitcoin-core-dev meeting
19:02:55 <jarolrod> Hi
19:02:58 <ajonas> hi
19:03:01 <sipa> hi
19:03:06 <fjahr> hi
19:03:32 <Murch> hi
19:03:35 <pablomartin_> hello
19:03:43 <laanwj> no meeting topics have been proposed this week, but we still have some from last week from achow101: Adding (more) maintainers as GitHub org owners, Moratorium on refactors that are not part of larger projects that bring tangible features and fixes
19:03:50 <laanwj> any last minute ones?
19:03:57 <furszy> hi
19:04:40 <jonatack> hi
19:05:09 <laanwj> #topic High priority for review
19:05:09 <core-meetingbot> topic: High priority for review
19:06:04 <b10c> hi
19:06:05 <laanwj> https://github.com/orgs/bitcoin/projects/1/views/1  6 blockers, 5 chasing concept ACK
19:06:08 <kouloumos> hi
19:06:09 <gleb071> hi
19:06:13 <laanwj> anything to add/remove?
19:06:39 <instagibbs> #26398 to replace #26265 please?
19:06:40 <gribble> https://github.com/bitcoin/bitcoin/issues/26398 | Relax MIN_STANDARD_TX_NONWITNESS_SIZE to preclude 64 non-witness bytes only by instagibbs · Pull Request #26398 · bitcoin/bitcoin · GitHub
19:06:41 <gribble> https://github.com/bitcoin/bitcoin/issues/26265 | POLICY: Relax MIN_STANDARD_TX_NONWITNESS_SIZE to 65 non-witness bytes by instagibbs · Pull Request #26265 · bitcoin/bitcoin · GitHub
19:07:38 <gleb071> i have #26359, it's small but still a blocker so may qualify i guess?
19:07:40 <gribble> https://github.com/bitcoin/bitcoin/issues/26359 | p2p, refactor: #23443 (Erlay support signaling) follow-ups by naumenkogs · Pull Request #26359 · bitcoin/bitcoin · GitHub
19:08:24 <laanwj> instagibbs: as blocker or chasing concept ack?
19:08:31 <gleb071> the second batch PR is in draft and need little care, hopefully will be ready for review next week, so this is a good target for now :)
19:08:39 <instagibbs> laanwj, ah, i guess remove chasing concept ack on old, add high prio to new
19:08:39 <laanwj> gleb071: sure, size isn't important for this
19:08:48 <laanwj> instagibbs:ok!
19:09:04 <instagibbs> thanks!
19:09:20 <instagibbs> gleb071, great seeing forward motion :)
19:09:50 <laanwj> gleb071: added
19:10:35 <gleb071> thank you! you can drop the "meta-issue" from the last column too; not sure it's useful anymore
19:11:02 <gleb071> the issue is hopefully useful, but i think the distribution strategy can be different now
19:11:42 <laanwj> right, it's no longer waiting for concept ack
19:12:17 <gleb071> apparently i can edit the table? okay
19:13:03 <laanwj> the permissions on the github projects changed a while ago
19:13:52 <laanwj> i mean, the project boards... they're no longer part of a repository, but global to the organization, and have their own perissions
19:14:10 <laanwj> you must be in a team allowed to edit it
19:14:35 <gleb071> got it
19:15:23 <jarolrod> sound like a good segway into achow101 topic on org owners
19:15:55 <laanwj> you're only in "frequent contributors" which should have only read access to high prio for review... that's kinda weird
19:16:31 <laanwj> segway hehe
19:16:50 <laanwj> #topic Adding (more) maintainers as GitHub org owners (achow101)
19:16:50 <core-meetingbot> topic: Adding (more) maintainers as GitHub org owners (achow101)
19:17:02 <jarolrod> for reference: https://docs.github.com/en/organizations/managing-peoples-access-to-your-organization-with-roles/roles-in-an-organization#permissions-for-organization-roles
19:17:08 <gleb071> laanwj: i tested editing very moderately to not cause some org-wide notifications so it's possible you're right. i'm not interested enough to look further into that, but i can if you wish
19:17:26 <achow101> speaking of permissions, of the current owners of the bitcoin and bitcoin-core orgs, only 1 is a current maintainer, the rest former
19:18:01 <achow101> so I think it would make sense to have some of the current maintainers be added as owners as well
19:18:13 <laanwj> gleb071: i don't particularly care either, i'm sure everyone in the org is responsible enough not to do stupid things with the project board
19:18:45 <achow101> aiui, the main thing org owners do is administrative, and clean up spammers
19:18:54 <jarolrod> if a maintainer is an owner, and drops commit access, seems sane to drop org ownership as well
19:19:06 <luke-jr> but we should have more than 1
19:19:13 <luke-jr> eg, in case someone gets hit by a bus
19:19:21 <achow101> luke-jr: yes, that's mainly why I bring this up
19:19:28 <vasild> is that list public?
19:19:31 <luke-jr> achow101: but they can potentially do a lot more than spammer-triage
19:19:52 <jarolrod> org moderators can clean up spammers as well btw
19:19:57 <achow101> vasild: maybe? for contributors already part of the org, you can see who the owners are, not sure how public though
19:20:38 <vasild> ok
19:20:42 <laanwj> no, this isn't public, and maybe it's important to be somewhat discrete about who exactly has owner permissions, for bus factors etc
19:21:09 <vasild> then this is not suitable for public irc meeting?
19:21:23 <vasild> s/not/semi/
19:21:23 <luke-jr> FWIW, lack of bitcoin org ownership prevented me from adding Kalle to the bips repo a while back, but I'm not sure I want that kind of access (in terms of becoming a possible target)
19:21:27 <laanwj> i'd agree adding one more maintainer would make sense though
19:21:37 <sipa> agree as well
19:21:47 <laanwj> not adding a maintainer but to the owners i mean
19:21:55 <luke-jr> laanwj: in terms of privacy, maybe it's best if there is no correlation with maintainers then?
19:22:12 <jonatack> gleb071: i don't have any additional access priveleges and it looks like i could edit the project board too
19:22:28 <achow101> I think it would make sense for maintainersto be owners as owners can (force) push to protected branches
19:22:44 <jonatack> vasild: yes, it doesn't appear in, say, https://github.com/orgs/bitcoin-core/people that frequent contributors can see higher access privileges
19:22:45 <achow101> and so any owner would be a committer, even if their key is not in trusted-keys
19:22:59 <laanwj> luke-jr: i don't understand, you mean no one of the owners was available or wanted to add Kalle for you?
19:23:11 <luke-jr> laanwj: no, I mean I had to ask an owner to do it
19:23:16 <laanwj> oh, sure
19:23:26 <luke-jr> which is fine IMO
19:24:06 <laanwj> yeah... it doesn't seem like something that often happens, anyhow
19:24:16 <achow101> I think knowledge of who the owners are is semi public already, as we all know who to ping to cleanup spam
19:24:32 <luke-jr> [19:19:52] <jarolrod> org moderators can clean up spammers as well btw
19:24:55 <luke-jr> so ownership of the org seems like a basically useless thing, but something we must maintain to avoid problems XD
19:25:04 <jonatack> achow101: yes, istm anyone following the repo closely can see who is cleaning up
19:25:27 <jarolrod> ^^ we don't have any moderators currently though
19:25:53 <glozow> achow101: to clarify you’re suggesting everyone in trusted-keys should also be org owners?
19:26:08 <luke-jr> jarolrod: I assume that's as easy to resolve as adding owners
19:26:17 <jarolrod> ^^ that would be too many owners
19:26:35 <jarolrod> i think he means of the owners, they should be a subset of current maintainers
19:26:40 <luke-jr> ^
19:26:44 <achow101> luke-jr: I think org owners are also the only ones that can add new members? and also migrate repos in/out of the org (e.g. we might want to move libmultiprocess into bitcoin-core)
19:26:56 <luke-jr> achow101: ah
19:27:07 <glozow> oh i see
19:27:09 <jonatack> i'd suggest achow101, providing you are willing
19:27:33 <jarolrod> i'd suggest a specific maintainer who has a robot :D
19:27:49 <achow101> I am willing
19:27:59 <vasild> +1
19:28:00 <luke-jr> +1 for achow101
19:28:13 <ariard> ACK achow101 for org ownership
19:28:24 <gleb071> ACK
19:28:38 <josie[m]> ACK achow101 for org ownership
19:28:39 <b10c> ach ackow101
19:28:44 <fjahr> ACK
19:28:49 <josie[m]> very game of thrones vibe: "I am willing"
19:28:51 <luke-jr> ach? O.o
19:28:55 <pablomartin_> ack
19:29:00 <jonatack> b10c: nice inversion
19:29:09 <jarolrod> 👍
19:29:55 <laanwj> ok... seems we have agreement on that, at least inside the meeting
19:30:06 <glozow> ack
19:30:50 <laanwj> #topic Moratorium on refactors that are not part of larger projects that bring tangible features and fixes (achow101)
19:30:51 <Murch> +1
19:30:51 <core-meetingbot> topic: Moratorium on refactors that are not part of larger projects that bring tangible features and fixes (achow101)
19:31:06 <sipa> Like with the question of adding maintainers, I think this is primarily a decision to be made by current maintainers/owners, as they have the best view on where help and of what kind is needed. Personally, ACK achow101.
19:32:26 <laanwj> yeah... but it's good to have broad agreement i guess
19:32:59 <achow101> We briefly discussed at coredev to reduce the number of refactors opened as they can have quite an effect on rebasing and review
19:33:36 <achow101> so perhaps we should stop opening refactoring prs that are not well motivated in the context of doing something larger that requires refactoring
19:34:11 <vasild> Such a moratorium seems like a blanket NACK on a vaguely defined set of changes. Who is deciding whether something is tangible?
19:34:23 <achow101> (this is both an effort to make it easier/faster to get things merged, and reduce the number of prs open)
19:34:36 <sipa> I agree it's pretty vague, and hard to decide anything about in general.
19:34:48 <sipa> But maybe you do have something more specific in mind (a few examples?)
19:34:54 <laanwj> it seems somewhat vague... maybe encourage people to nack specific refactors they disagree with?
19:35:09 <luke-jr> seems more like a vague suggestion
19:35:10 <instagibbs> or encourage people to state up front goals in refactoring PRs
19:35:20 <luke-jr> NACK is too strong - more like just silence on the PRs :P
19:35:52 <achow101> I suppose this is more of a "state your motivations" kind of suggestion, and perhaps think before refactoring
19:36:02 <laanwj> no i mean really nack, as in "yes it's a minor improvement but it's not worth it to change this many files" and things like that
19:36:03 <josie[m]> instagibbs: +1 , i think making a compelling argument for why you are doing something, and how it impacts further work is always a good idea
19:36:04 <_aj_> "-0 -- what is this building towards?" or something
19:36:29 <vasild> I think the current scheme works well: 1. if somebody bothered to write the code and 2. enough people think it is important enough and bother to review it and 3. some maintainer decides to merge it => then it is ok to have it in master
19:36:35 <sipa> I think it's generally helpful if there were more concept acks/nacks, even ones motivated as "unclear motivation".
19:37:01 <brunoerg> vasild: +1
19:37:06 <sipa> (certainly something I'm guilty off to, I prefer to stay silent on things I don't strongly dislike...)
19:37:07 <gleb071> The only two clear things we can do here is to say is to hear. I do share the opinion sometimes the refactorings are not justified given our review capacity. I will do my part in not producing such refactors.
19:37:20 <achow101> one example of such a refactor was a getInt thing in unvalue(?) from a few months ago that caused rebasing for no clear reason
19:37:40 <jonatack> it's hard to see this being more than a general encouragement, given the ad hoc nature of how we (prefer to work), but in general  if the why doesn't make sense then the what probably doesn't matter, so that is always a good reason to well motivate PR descriptions
19:37:45 <gleb071> "is to say and to hear" i meant...
19:37:46 <luke-jr> achow101: I did NACK that, and got ignored
19:37:51 <lightlike> what is the intention behind "moratorium"? Disallow them periodically in a certain time span for each release cycle, or disallow them indefinitely "for now"?
19:38:12 <josie[m]> luke-jr: i kinda wished more people would NACK rather than be silent, so long as they take the time to explain why. it can be kinda frustrating with just silence because you dont know if people disagree or are just too busy
19:39:01 <luke-jr> josie[m]: I'd put refactoring moratorium as "just too busy" more than "disagree" IMO
19:39:36 <achow101> lightlike: indefinitely "for now"
19:39:57 <gleb071> We can think of inventing a new means of communication. Say, if you lack reviewes, you explicitly tag relevant devs. Then they feel better giving low-prio-nack.
19:41:35 <achow101> mainly this topic was to seed the ideas of either stop opening refactors, or be more agressive aboue nacking things
19:41:43 <vasild> The section "### Refactoring" in CONTRIBUTING.md is relevant to this and IMO is very well written
19:42:41 <ajonas> My concern is that a lot of the refactors that you may have in mind are authored by people not at this meeting. I think the best way to enforce would probably be to leave comments pushing for better motivation or NACKing (which requires work) and wait for the next kill, shill, merge session. Over time, that gets absorbed though it will also turn many newcomers away.
19:43:33 <jonatack> lightlike: there have been proposals in the past to concentrate refactorings at certain parts of the release cycles, but that didn't go further for the same reason as here (probably): an ad hoc approach seems more conducive to this open source project than top-down project management
19:43:39 <laanwj> newcomers really shouldn't open refactors :)
19:43:50 <_aj_> ajonas: (kill/shill/fulfill?)
19:43:59 <laanwj> that takes understanding of the code and purpose... we have that documented in CONTRIBUTING.md even
19:44:07 <josie[m]> aj: nice
19:45:19 <ajonas> laanwj: no argument from me.
19:46:27 <josie[m]> ajonas: i think this is a good point, and in any case, being more communicative on PRs can't hurt. im not sure about turning newcomers away tho. at least from my experience, the silence can be more discouraging then someone giving a (constructive) NACK
19:47:02 <laanwj> we used to have a lot of out of the blue refactores, which prompted that to be written, it's not a new issue really but also a complicated one
19:47:17 <_aj_> maybe point newcomers more at writing additional test coverage for existing code?
19:47:34 <luke-jr> I noticed earlier a dumb refactor simply closed with no comment. I imagine that's pretty discouraging <.<
19:48:01 <laanwj> josie[m]: i definietly agree to that (from my own experience of outside contributor to projects)
19:48:09 <luke-jr> https://github.com/bitcoin/bitcoin/pull/26374
19:48:25 <laanwj> "no i don't want this" is better than just silence for months or it lingering for years even
19:48:32 <jonatack> maybe a canned response can be given to newcomer refactors. that said, from what i've seen when a newcomer proposes an intelligent|useful cleanup or refactoring, it not only tends to be well-
19:48:58 <jonatack> received but also perhaps gets attention and merge more quickly
19:49:30 <jonatack> which could encourage more refactoring perhaps
19:49:42 <fjahr> adding tests is good but also fixing actual bugs, there are enough open issues. But for a newcomers that is probably a lot of work and many seem to look for instant gratification.
19:49:46 <laanwj> as a contributor you're generally aware not everything will be merged and there can be disagreements
19:50:53 <jonatack> as refactoring outwardly may seem easier to review as well and divert review attention (as mentioned a bit earlier above in the meeting)
19:51:05 <josie[m]> laanwj: +1
19:51:34 <laanwj> fjahr: hah yes... a long time ago there used to be this awful system that paid out per commit... this encouraged a lot of people to try to get *something* into bitcoin core, the most trivial and cosmetic changes
19:52:06 <sipa> that was terrible, i remember
19:52:14 <josie[m]> jonatack: i think the fact that refactors appear to be easy can also make them very dangerous, as in they might not get as thorough a review there is a non-zero chance they introduce subtle bugs
19:52:20 <achow101> jonatack: even "useful cleanups" are detrimental as they can cause conflicts with other things and delay other PRs from being merged
19:52:23 <luke-jr> I think we did lose a developer over that being shutdown tho
19:52:27 <_aj_> luke-jr: seems like a better pr than solidity got https://github.com/ethereum/solidity/pull/13646 at least
19:52:36 <gleb071> hacktoberfest
19:52:44 <jonatack> josie[m]: achow101: agree
19:52:47 <kanzure> does gh-meta capture comment edits?
19:52:51 <luke-jr> _aj_: hahahah lol
19:53:06 <ajonas> fjahr: strongly agree here but really hard to find the right entry point. Good first issues has produced varied results.
19:53:07 <sipa> kanzure: I think so.
19:53:15 <laanwj> kanzure: it should
19:53:29 <sipa> luke-jr: I do believe we lost one somewhat-useful contributor over this.
19:53:44 <sipa> Who got really demotived when told to reduce the number of commits in PRs.
19:53:49 <sipa> *demotivated
19:53:50 <jonatack> josie[m]: indeed, refactors have been a real source of non-trivial bugs
19:54:08 <vasild> "that was terrible, i remember" -- it can be worse, it can pay per number of added lines.
19:54:32 <laanwj> vasild: lol... someone might add a flight simulator!
19:54:36 <sipa> jonatack: Do you have an example? (not disagreeing, just kind of wondering what you're thinking of)
19:54:41 <vasild> yes :-D
19:54:52 <sipa> I think we need built-in tetris in the GUI.
19:54:57 <sipa> As RNG source, or something.
19:55:04 <laanwj> hehe
19:55:21 <laanwj> mempool tetris
19:55:23 <vasild> hmm, actually it may make sense to pay per _removed_ lines (off topic, sorry)
19:55:26 <josie[m]> id start contributing to the bitcoin GUI if we had built in tetris :D
19:55:30 <kanzure> coin selection and tetris are both examples of the same class of problem
19:55:34 <ajonas> _aj_: (kill/shill/thrill?)
19:55:40 <jarolrod> sipa: that can be accommodated
19:55:40 <luke-jr> vasild: [19:48:08] <luke-jr> https://github.com/bitcoin/bitcoin/pull/26374
19:55:50 <_aj_> ajonas: thrill is when it gets mentioned in an optech newsletter
19:56:05 <jonatack> sipa: refactorings touching the init order bring to mind a couple
19:56:45 <laanwj> yes init order (especially in the gui, as there are lots of hard to test edge cases) is really dangerous to change, agree jonatack
19:56:48 <kanzure> is the concern about the set of open PRs being too plentiful, or that an old refactor becomes virtually useless over time due to staleness?
19:56:57 <_aj_> oh, i have a question about init (shutdown) order. can wait to post meeting in a few mins
19:57:47 <luke-jr> inb4 _aj_ refactors init/shutdown order for no reason (jk)
19:57:55 <jarolrod> Play Tetris while core syncs
19:57:59 <laanwj> kanzure: i was thinking of transaction selection for blocks, but coin selection works too
19:58:08 <_aj_> nah, it's just what seems like a bug
19:59:49 <jonatack> laanwj: i think i recall one causing UB and a re-release (nov/dec 2020?) and one causing an addrman corruption issue (july/aug 2021)
20:00:21 <laanwj> jonatack: yes, you'er right
20:00:30 <laanwj> ok, time to close the meeting
20:00:33 <laanwj> #endmeeting