Home < Bitcoin Core Dev Tech < Bitcoin Core Dev Tech 2023 (Apr) < Refactors

Refactors

Speakers: Fabian Jahr

Date: April 25, 2023

Tags: Bitcoin core

Category: Core dev tech

One take-away from the Chaincode residency in 2019 was: Don’t do refactors (unless you really need it)

A marked increase from 2019 to today (Chart on the increase of refactors)

The comments and PRs are steady but the refactors are increasing

Quibble about how regular reviewers are counted (should be higher than 5 comments)

Project reasons:

  • Ossification?
  • Natural way mature projects progress/Boy Scout Rule

Personal reasons:

  • Time commitment of large review may not be possible (extended period of singular focus)
  • Merged PRs as proof of work, could help with the next grant
  • Feels good to ship

What is the effect?

  • Working on refactors takes away from other work
  • Leads to rebases, sisyphus work for authors and reviewers
  • Speeds up for new-comers but slows down experienced contributors
  • Renamed variables as example

Refactoring has been fully commoditized with LLMs

  • If there is a good refactor from GPT, we should take that too

PRs from newcomers

  • We allow for less complex topics

Part of the problem is too much politeness

  • You don’t have to ignore it
  • It does have value
  • But it depends on whether it’s a problem
  • In 2014: people have also been complaining about paying too much attention to small things
  • If reviewers have preference for picking smaller things to look at first, that also detracts
  • This is probably a problem because of reviewers choosing to review smaller things
  • Harsh feedback scares people away
  • More pushback on the individual level

If you are part time, it’s easier to do small things

  • Full timers - more comfortable to jump into something that is deeper - we don’t have
  • Like to work on larger domains but grants may not value that as much
  • Open a lot of smaller refactors to make it seem like they are doing something

High value refactors:

  • Fuzz targets
  • Some refactors that are high value that never get addressed
  • Significant PRs can have refactor commits and sometimes those are the best kind
    • The reason why the big projcects have refactors is because it is addressing the technical debt
  • We need to be more proactive and vocal about what we want and don’t want

Onboarding perspective - if you are mentoring someone, be mindful for what they are working on

Funding matters when it comes to diving into something deeper vs nights/weekend contribution

Don’t have enough good first issues anymore, label isn’t used much lately We should give better guidance to newcomers and those that may be experienced but don’t have much time “I don’t think we should have follow-ups to fix your nits” “If you are the 3rd person to ack a PR and you leaving 30 nits, then it probably doesn’t matter”

Q: Did the refactor moratorium ever have any effect?

A: It was never instituted

Q: Do we have any guidance on the contributing guidelines?

A: Yes

Q: Do nit changes need to invalidate ACKs where the maintainers can clearly see that they don’t impact functionality?

A: The merge script won’t pick up something from the older commits

Q: Could change the merge script to add the stale acks, but the commits they ack disappear

A: If you want attribution, you can re-ack

You are allowed as an author to ignore nits, but it’s good for you to acknowledge them, even if you are going to ignore them