[HN Gopher] Ship / Show / Ask (2021)
___________________________________________________________________
Ship / Show / Ask (2021)
Author : thunderbong
Score : 22 points
Date : 2024-01-21 15:16 UTC (7 hours ago)
(HTM) web link (martinfowler.com)
(TXT) w3m dump (martinfowler.com)
| ivraatiems wrote:
| I'm confused by this. I've never worked in an organization where
| "commit directly to main" would get you anything but in a ton of
| trouble.
|
| Is that a normal/common practice for anyone here?
| lijok wrote:
| Yes, in loads of shops. See trunk based development.
| perryizgr8 wrote:
| Trunk based development involves creating short lived feature
| branches, not committing directly to master. At least I
| haven't seen it done that way.
| lijok wrote:
| You're describing scaled trunk based development
|
| TBD works on main, or "trunk" as described in the spec
| izacus wrote:
| Can you list some shops that follow this.... "spec"?
| lijok wrote:
| Scaled TBD or TBD?
| hipadev23 wrote:
| Super common. I'm not sure how that's any different than teams
| who create a PR, beg someone to review it, they glance it for 3
| sounds and leave a "LGTM!" approval so you can finally merge to
| main.
|
| Same result, whole lot less performative bullshit.
|
| We use branches and reviews for large changes. Small scope
| changes go directly to main. If it breaks, we rollback and tell
| that person to stop breaking things.
| StreetChief wrote:
| What if a pr was actual code review and not performative
| bullshit?
| esafak wrote:
| It only breaks in staging, right? I hope you're not running
| anything in the physical world like Uber or Tesla.
| lmeyerov wrote:
| `lgtm` is a legit typical outcome. biasing to approving, in
| many companies & software setups, is a great default. ex: we
| might `lgtm`, or make comments that are suggestions for
| follow-on PRs without blocking the current.
|
| However... it's also incredibly hard to self-edit and
| accurately `lgtm` yourself
| bootsmann wrote:
| 4 eye principle brings a lot of redundancy. Even if someone
| just goes lgtm, it prevents people from pushing a change that
| no other person has seen which means if the original engineer
| is on holiday you still have someone who is somewhat familiar
| with the code.
| dharmab wrote:
| I've seen this approach on smaller teams working at private
| companies in less-regulated industries. Think a team of 3-5
| devs working at a small company.
|
| At publicly traded companies or in regulated industries, yeah,
| your compliance will require you to disable merging without at
| least one approval.
| lijok wrote:
| This is good advice to surface the way we work, but I don't see
| why you would push to main for Ship rather than raising a PR that
| you merge immediately (Show), if you're using PRs anyway.
| alexchamberlain wrote:
| I would like to see a more pragmatic approach to PRs than the
| Github esque red barrier of doom you see when you enable branch
| protection and minimum number of reviewers. I trust my team to
| judge if their change is trivial enough that it doesn't need a
| second review. As some others have said, if you're using PRs
| _sometimes_, the cost of running tests even for trivial changes
| is minimal, so I'd lean towards show and ask, rather than ship.
|
| That being said, I wasn't massive fan of the comparison to
| continuous integration. At least, I've never used it in that
| sense. To me, continue integration is the practice of
| continuously testing your main branch against your dependencies
| and always testing changes when they are made. Ideally,
| dependencies are continuously updated too (at least in the case
| of apps). This is somewhat orthogonal to the code development and
| review practices that lead to code being on the main branch in
| the first place.
| dang wrote:
| Related:
|
| _Ship / Show / Ask: A modern branching strategy_ -
| https://news.ycombinator.com/item?id=28510212 - Sept 2021 (149
| comments)
|
| _Ship / Show / Ask - A modern branching strategy_ -
| https://news.ycombinator.com/item?id=28457071 - Sept 2021 (3
| comments)
| theusus wrote:
| This article feels a lot patronizing than anything useful.
|
| When you say "changes so big" you think your solution is better.
|
| I am working in an organisation with so called "seniors" and
| their PRs are approved because other seniors made them friends.
| chrsig wrote:
| > But the adoption of Pull Requests as the primary way of
| contributing code has created problems. We've lost some of the
| "Ready to Ship" mentality we had when we did Continuous
| Integration. Features-in-progress stay out of the way by delaying
| integration, and so we fall into the pitfalls of low-frequency
| integration that Continuous Integration sought to address.
|
| Feature branches should be pulling/rebasing back on top of
| master, so the integrate smoothly into main.
|
| > Sometimes Pull Requests sit around and get stale, or we're not
| sure what to work on while we wait for review.
|
| This is a sign to me that there's something going on along the
| lines of:
|
| - the work isn't actually necessary / no one asked for it
|
| - there are more pressing issues demanding others attention, and
| it's ok that it's sitting there for a while
|
| - the team doesn't feel confident that their comments will be
| well received
|
| - the team is dysfunctional and unable to communicate the need
| for a change to the PR
|
| - the team doesn't understand review is one of their
| responsibilities
|
| - the team is doing sprints, fills their sprint up with 40
| hours/person of development time, then forgets to ponder where
| time for review will come from.
|
| Fix the team communication problem, or anything else you do is
| also doomed, you just don't know how yet.
|
| > Sometimes they become bloated as we think "well, I may as well
| do this while I'm here."
|
| So it hasn't gotten reviewed, may as well make it harder to
| review, that'll get it somewhere...
|
| Once it reaches a certain size, you need to take a step back and
| consider how you can chunk it out to make it reviewable
| incrementally. If you can't do that, examine how it can be
| factored differently to allow incremental review.
|
| > First - a trick to help you get the best of both worlds - merge
| your own pull request without waiting for feedback, then pay
| attention to the feedback when it comes.
|
| Why are people in such a hurry to get it merged? If it's so
| trivial that it doesn't require a review, it's likely not so
| impactful that good engineering practice should be ignored.
|
| Without review, I promise that the quality / prevalence of tests
| will decrease over time, and eventually you wont be able to trust
| that the one line change that you're making wont break something
| in production.
|
| Patience and rigor help throughput. This article seems like it's
| advocating skipping around those because they require
| communication and get in the way of "getting things done" a. la.
| cowboy.
| srvaroa wrote:
| This.
|
| Most of the articles about working around code reviews give the
| same vibes as naive crypto enthusiasts towards financial
| regulation.
| danielovichdk wrote:
| I can't tell you what it is but I have such a hard time reading
| content on Martin Fowlers site.
|
| I might be he tries so hard to make things his own. Is there
| anything he does not have an opinion about ?
| Jtsummers wrote:
| > I might be he tries so hard to make things his own. Is there
| anything he does not have an opinion about ?
|
| Fowler isn't the author of this content, Rouan Wilsenach is.
| chrsig wrote:
| Honestly, that's always made it (content on martinfowler.com)
| a bit weirder to me.
___________________________________________________________________
(page generated 2024-01-21 23:01 UTC)