8
Crost
3y

Are PRs only to help juniors? I don't think so but someone I know does.

Comments
  • 5
    PRs are good for implementing a mandatory code review workflow.
    One use case is huge open source projects with lots of untrusted contributors (like that famous kernel managed by the creator of GIT).

    If you don't have a mandatory code review workflow (so code is always accepted anyways), pull requests are just a useless extra step.
  • 6
    This is assuming non junior level developers are perfect in their implementation and never introduce bugs.
  • 1
    @Oktokolo what's the alternative? Cheery-picking up the co-operate tree of branches?
    I'd rather PR and let git ensure the diffs are in sync...
    Plus it's a nice ref. When things blow up anyways.
    Gives you a scope of things to traverse
  • 4
    @lotd Sending patches by email. This is the "origjnal" git workflow.
  • 1
    @sbiewald ah yes, the good ol' days 🥲
  • 2
    Obviously not, It's to get more eyes on a solution
  • 1
    I would summarise that their purpose is to share responsibility and knowledge
  • 2
    Nope, we all make mistakes and it's educational to get comments and give comments
  • 1
    @sbiewald Oh yeah. The good ol' "I emailed you the last version. just upload to production! all is good!".

    PR should be done for all changes - Worst case scenario: get feedback, and learn new things. Or in the best case scenario: prevent WW3.
  • 1
    @molaram well it's opinion based
  • 2
    @magicMirror Of course changes must be discussed, but this was not what I meant. Git has subcommands for sending patches as emails.

    The sender creates a patchset with "git format-patch", sends it with "git send-email", the patch is then discussed and possibly applied with "git am".
    This workflow is "still" used for Linux kernel development and git itself¹. See https://lore.kernel.org/bpf/... for an example.

    A pull request was used for larger changes with an own public git repository. The email message was formatted with "git request-pull".

    ¹: While both projects do accept PRs, sending patches as emails is still common and at least for Linux preferred.
  • 5
    If you don't have a form of code review and commitment workflow as a team, please burn the project down and end the codes misery.
  • 1
    @sbiewald linus does not want idiots to spam his git tree.
    So it makes sense to work like this.
  • 1
    If so, call me a junior
  • 1
    Reviews are mandatory for everyone.
  • 5
    Not at all. I've been called out for stupid stuff I've done by juniors more times than I'd care to mention.

    We have PRs because stupid mistakes happen and a second pair of eyes always helps - no matter the experience.
  • 0
    @lotd
    The alternative for when there is no code review requirement is to:
    Fetch upstrem, rebase your dev branch to upstream master, merge dev branch to master, do a last quick testing (including unit tests), and then push master to upstream.

    @Haxk20
    Well, you can do kernel development with pull requests. Linus, despite having created that feature, isn't using them for his famous kernel project...
    But it still is a pretty obvious use case.
  • 0
    @IntrusionCM
    I guess, you have never been the only dev of a company.
  • 1
    @Oktokolo ... I wrote as a team explicitly.

    Though I admit that in my opinion having only a single coder is a bad idea TM.

    After all, we're humans and make mistakes.

    Single coder usually means one person has written the entire codebase entirely to his liking without any form of criticism.

    Meaning the codebase is most likely a mess.

    Note the word usually - I've seen a few codebases from a single coder who were enjoyable but that's a rare phenomenon.
  • 1
    @Oktokolo that's a simple nice & setup...
    For me it would be
    From $org-$me/$proj:$feature-sprint-{1,2,3}
    To $org/$proj:$feature-{dev, staging, qa, prod}
    Then $org/$proj:main

    Usually those branches are quite far apart and local merges tends to result in unexpected changes..
    At least with a PR that kinda narrows the scope to hunt in down..
  • 1
    @IntrusionCM
    Sorry, completely missed the team constraint.
    And you are right: Being in a team is better than having to do it alone.

    @lotd
    Use GIT's rebase command to keep your branch from getting behind. I fetch daily and if there are changes, i rebase them into my dev branch immediately.
    GIT really makes the whole concurrency aspect of software development so unbelievably easy - it almost feels like cheating (especially when you used Subversion before).
  • 1
    I use PRs on my home project where I'm the only dev. GitHub is set up to do a series of tests on every PR. Sometimes (because I fuck up) tests are successful locally, but not in the pipeline. This way it's more convenient for me.
  • 1
    @Oktokolo you mention subversion and I raise you... Perforce.
  • 0
    @KatatonDzsentri
    never used anything from them.

    Did they made the best version control system before Linus made GIT?
Add Comment