> "Pull requests were invented to gatekeep access to open-source projects. In open source, it's very common that not everyone is given free access to changing the code, so contributors will issue a pull request so that a trusted person can then approve the change. "I think this is really bad way to organise a development team. "If you can't trust your team mates to make changes carefully, then your version control sy…
Just a small note: "trust but verify" is not a reasonable statement, and it was also unreasonable in its original context, which I encourage anyone to research. If you can verify, trust is not required.
I think it is usually used in context of "we don't verify because we don't trust you but because mistakes always happen".
Calling pull requests "pull request " when it is a job to create a change is what's wrong. The author isn't actually requesting anything. But what else could be a nice short term for "collection of changes with technically enforced reviews and tests"?
The problem is again, Github
Originally, "pull request" was exactly what name implied, someone asked you to pull from their repository. The original Git collaboration model was that, you pulling changes from other repos and integrating them in your own, vs handing merge request.
Github co-opted the term to mean exact opposite, accepting essentially push from different repo.
Up to about 10 years ago I used to work on projects where up to 100 people (of those about 20 coders, the rest artists and designers) committed directly into the main branch without a formal review process, and it was ok (this was with SVN though). Code review happened informally "after the fact", usually as the first thing in the morning when updating (each person simply looked through the changes of other people in…
> I cannot imagine how this scenario could work with git though.
exactly the same as with SVN ? The one feature I could see it lacking is ability to lock file in SVN but otherwise "single master repo" work on SVN and Git is very similar
Works absolutely fine with git, you just have to take the time to keep the pre-commit checks reasonably quick
IMHO what makes it harder in git (for many people committing into the same branch) is that each team member has its own local history which needs to be synchronised via push and fetch (which quickly gets out of control for 'non-technical' team members). This separate step doesn't exist in centralized version control systems like SVN where everybody is always on the same timeline. Working exclusively on branches and t…
You can make it do automatic rebase on pull to make it look more like SVN single history.
We do that on our CI repo because of many tiny changes of unrelated parts that otherwise just made almost every second commit into a merge
> "Pull requests were invented to gatekeep access to open-source projects. In open source, it's very common that not everyone is given free access to changing the code, so contributors will issue a pull request so that a trusted person can then approve the change. "I think this is really bad way to organise a development team. "If you can't trust your team mates to make changes carefully, then your version control sy…
> PR is about verifying This is interesting as a counterpoint to trusting, but it’s funny, I’ve never thought about PRs in either of those terms, not within a team or company anyway. The Pull Request can also be about the code writer asking for review, asking the question ‘is this generally the right way to go?’ The funny thing about projecting ‘trust’ and ‘verification’ onto the process is that PRs simply do not pre…
I've had groups where I eventually 'approved' a PR, because... deadlines... even when it wasn't meeting defined standards (primarily around tests/docs). Comments like "please add a test" or "please document X" were just ignored (both inside the version control system and in slack). Eventually, I would merge, and occasionally, we'd hit some bug, trace it back to that, and I would catch blame for "passing" it. Not the person who wrote it, not the same person who didn't test it or add any automated tests. I did, even though I was doing so under protest. The couple times I added to their PR - added a test to verify behaviour, for example - I got called out for "stepping on others' code". Apparently if you went out on smoke breaks with 'the guys', they were 'cool' wit h you and that was the political way of getting people 'on your side' to go along with stuff. Very ... odd dynamics. Apparently, if you're dating the dept head's niece, writing tests was optional, but I never got that memo.
> PR is about verifying This is interesting as a counterpoint to trusting, but it’s funny, I’ve never thought about PRs in either of those terms, not within a team or company anyway. The Pull Request can also be about the code writer asking for review, asking the question ‘is this generally the right way to go?’ The funny thing about projecting ‘trust’ and ‘verification’ onto the process is that PRs simply do not pre…
I've had groups where I eventually 'approved' a PR, because... deadlines... even when it wasn't meeting defined standards (primarily around tests/docs). Comments like "please add a test" or "please document X" were just ignored (both inside the version control system and in slack). Eventually, I would merge, and occasionally, we'd hit some bug, trace it back to that, and I would catch blame for "passing" it. Not the…
Simple solution to that is don't pass it. When the deadline comes, you'll have the reason documented.
> "Pull requests were invented to gatekeep access to open-source projects. In open source, it's very common that not everyone is given free access to changing the code, so contributors will issue a pull request so that a trusted person can then approve the change. "I think this is really bad way to organise a development team. "If you can't trust your team mates to make changes carefully, then your version control sy…
>trust but verify If you have to verify, is there really trust? We need to get to the point that a lack of trust isn't some inherently bad thing. I don't trust myself to never make mistakes. A PR is a way to get others to add a layer of verification to help catch bugs. I don't even trust the PR to catch all bugs, but it is a cost effective for the amount of bugs it does catch. Why is this lack of trust a bad thing? H…
In this context if you arent trusted then your PR isnt even looked at.
It seems like we are reaching the same conclusions as Dave Farley on our development team. We have pivoted to just let everyone merge in their stuff and do "reviews" retroactively when we stumble upon code not up to the standard we set. The reasoning behind this is exactly that we trust individuals on the team to merge. and because reviewing in the traditional sense seems to be a formalisation of distrust: Something…
I would certainly fail my SOC2 audits if I told the auditor that anyone on the team can merge in any unreviewed change and that gets pushed to production, but not to worry be cause we'll catch any issues eventually. I trust my peers, but we all make mistakes and we do code reviews because we understand that working together can catch issues sooner. We aren't overly picky or mean during code reviews, but we make sure…
Seems like you are at a place where you need to be SOC2 compliant. Good for you that you are able to satisfy those requirements.
> "Pull requests were invented to gatekeep access to open-source projects. In open source, it's very common that not everyone is given free access to changing the code, so contributors will issue a pull request so that a trusted person can then approve the change. "I think this is really bad way to organise a development team. "If you can't trust your team mates to make changes carefully, then your version control sy…
> PR is about verifying This is interesting as a counterpoint to trusting, but it’s funny, I’ve never thought about PRs in either of those terms, not within a team or company anyway. The Pull Request can also be about the code writer asking for review, asking the question ‘is this generally the right way to go?’ The funny thing about projecting ‘trust’ and ‘verification’ onto the process is that PRs simply do not pre…
As someone who works in (and leads) the development of a 100k LOC project with four developers (project lead, tech lead and two junior developers) I see pull requests as a way for other members (often me or tech lead) to 1) spot clunky code 2) spot things that already exist in the code and should be unified 3) a way to teach and be taught about how we want the code to work.
I know that "my" team members look at PRs as more of a safety net for oopsies than evidence of lack of trust between team members. We allow self approval of PRs so that if/when someone is the only developer working they can still push code if needed.
When I started in the project we did not use PRs and we all agree that they have made the code better and that they have unified our coding style (the fact that Visual Studio/ReSharper auto-formats changed code also helps with that aspect).
> "Pull requests were invented to gatekeep access to open-source projects. In open source, it's very common that not everyone is given free access to changing the code, so contributors will issue a pull request so that a trusted person can then approve the change. "I think this is really bad way to organise a development team. "If you can't trust your team mates to make changes carefully, then your version control sy…
A PR is just a properly scoped nugget of review-ready changes. A formalized review process helps way more than it hurts in the long run.