• Pup Biru@aussie.zone
    link
    fedilink
    English
    arrow-up
    1
    ·
    edit-2
    3 hours ago

    i’d say it’s a balance… you’re totally right that individual requests for review should be relatively small (mostly so that they can all fit in your head at once), but imo equally valid is that everything in main should be a complete feature/fix: if you were to be gone immediately after merging, would someone need to continue or revert the change? would there be unused code laying around?

    this is where merge trains and a decent UI around them comes in handy: your main work is on a branch many small PRs each reviewed individually merge into that branch, and then when you’re done pretty much just automated integration tests, lint, and you’re good to merge the whole

    but equally some people prefer to solve this with things like gitflow, or just not at all and accept that main is always in flux

    reflector to support a new feature is also tricky: does it belong with the feature because it’s unnecessary abstraction without it? or is it its own PR because it stands in its own? and if it’s its own PR then how do you base your own feature branch on it before someone reviews and merges? how do you know you’re done without finishing? what if your assumptions are wrong and you need to try something new - just a lot of unnecessary churn and review?

    dev is messy and as always LOC is a pretty useless metric… keeping things understandable is key, and somethings a 17k line PR is the cleanest way to proceed