8

Senior dev:- "Limit your commits to have maximum 10 changes"..

Also senior dev:- Doesn't approves pending merge requests for days...

Comments
  • 6
    Adding translations across 42 files:
    > Translation commit 1 of 5
    > Translation commit 2 of 5
    > …

    Just. Why?
  • 6
    Seems relevant
  • 4
    "Limit your commits to have maximum 10 changes"

    This is a valid point though. That feeling you get when you gotta review a merge request with 133 changes is soul-crushing.
  • 2
    Did you mean max 10 changes per PR? Otherwise I don't see how it's related to the approval. Reason I ask is because I also see value in keeping a commit working on its own in case of cherrypicking, or making it possible to navigate through a larger pr at all.

    In any case, seniors not reviewing / approving must be a pita
  • 1
    Changes is number of files modified right? (Seems gitlab counts it way).

    That is just bullshit. It's more about the scope of work. A lot of my PR/MR's have only 3 changes or less but quite a few span in the 20 region. New feature need API change (openapi spec + route), model change. schema migration, Interface changes, tests, infra changes (cloud resources e.g. pubsub/bucket, add flag to kuberntes e.g env var in deployment and config maps) and then there is the actual implementation of the feature. They should be separate commits but the PR should be able to be checked for completeness and the functionality tested.
Add Comment