The Lounge is rated Safe For Work. If you're about to post something inappropriate for a shared office environment, then don't post it. No ads, no abuse, and no programming questions. Trolling, (political, climate, religious or whatever) will result in your account being removed.
I just can't get myself to agree with seemingly accepted mantra that very change / ticket implementation should go through a pull request...
Would be happy to know other people's opinion!
As a side note, there are many things that I have no clue when reviewing a pull request (like the business process behind) but there is also the fact that we have a developer here who is the best at writing horrible code, and just cant seem to understand the difference between a dictionary and a list.... I mean I just give up with that guy.
I cant complain too much though, he might be the reason why I got a juicy contracting job here, I sometimes think!
The only reason we do everything via pull-request is that this enables us to keep the master from direct changes (in case that the change breaks everything apart)...
So you can not commit directly to the master, only via pull-request, however you can approve yourself... (and the pull-request automatically triggers a build, as a condition to the merge)
"The only place where Success comes before Work is in the dictionary." Vidal Sassoon, 1928 - 2012
It's just another tool that can either be used to great effect or as unnecessary clutter. Depends on how the source control is managed.
I worked on a project where pull requests were used for a couple purposes:
1) to ensure merges into critical branches like main-development, production, and releases are explicitly done (not accidental),
2) as a form of code review where a pull request had to be accepted by at least one peer (you couldn't ok your own PR) after filling out a basic checklist (new tests pass, target branch tests still pass, merge is a fast-forward, and a couple others),
and 3) guarantee that git best practices are followed to avoid elephanting the history of the important stuff like ensuring you've merged main-dev into your branch and confirmed nothing broke before fast-forward merging back into main-dev. Fixing a merge problem on an important, shared branch, especially if it isn't caught until a couple merges later, is a nightmare.
It also serves as a discussion/checklist history so if anything does go catastrophically wrong, you not only know who requested the PR but who approved it and can educate them on what went wrong so it doesn't happen again.
You don't really get any of that with plain merges and I'd rather deal with PRs than paperwork. That's my 2 cents. Also I agree with everything Kornfeld said
1.) developer, well, develops
2.) developer tests
4.) PR approved, merge to master
5.) QA tests, UAT by client if needed.
It has been my experience that inflated egos do not jive well with code reviews(PR Reviews). To be honest, it took some time for me to get past my ego, in order to let someone else, whom I may not like, look at my code, and critique it, in the betterment of the product, not my ego.
I thought the same, but working on lqarge projects with 120+ people working on it 24/7 I appreciated the better traceability Pull Request give, because there are less of them.
Uusually we have tons of commits, hundreds of pushes and dozens of pull requests. In the PR we can / have to attach the code static analysis, link the issues related (if any) and have it approved by one or more people.
The integrator does not have to enter knee deep in our commits, he just takes the pull request, merges it and sends to validation. This allows for easier tracing of "approved, definitive changes" and easier regression tracking.
On smaller projects it may be too much, but you never knwo when a project blooms (or explodes).
GCS d--(d+) s-/++ a C++++ U+++ P- L+@ E-- W++ N+ o+ K- w+++ O? M-- V? PS+ PE- Y+ PGP t+ 5? X R+++ tv-- b+(+++) DI+++ D++ G e++ h--- r+++ y+++* Weapons extension: ma- k++ F+2 X
Last Visit: 31-Dec-99 19:00 Last Update: 25-Jan-21 14:44