Ranter
Join devRant
Do all the things like
++ or -- rants, post your own rants, comment on others' rants and build your customized dev avatar
Sign Up
Pipeless API
From the creators of devRant, Pipeless lets you power real-time personalized recommendations and activity feeds using a simple API
Learn More
Comments
-
Root797773yAdding translations across 42 files:
> Translation commit 1 of 5
> Translation commit 2 of 5
> …
Just. Why? -
isurunix693y"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. -
jaylord4513yDid 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 -
hjk10156963yChanges 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.
Related Rants
Senior dev:- "Limit your commits to have maximum 10 changes"..
Also senior dev:- Doesn't approves pending merge requests for days...
rant
wk275