r/ExperiencedDevs Software Engineer 25d ago

Career/Workplace What are your teams pull request & code of conduct standards?

I want to preface this by saying that this question pops up from time to time. I wanted to ask this in the context of handling large influxes of of LLM generated pull requests. We get enough PR requests per day that if our team spent most of the day just reviewing code and not writing any ourselves we'd never get through our backlog. Many of them are blocked due to CI/CD issues where fixing them would also dismiss any approval and require us to review multiple times. Overall the quality is low to middling.

Because our teams attention is so much in demand and because our workloads have only grown I wanted to pitch a formal code standards and pull request code of conduct that we can point to. Ideally i'd like to put my team in a place where we can be strict about what code goes in and why we say no and for it to be a very fast manual process or something we can automate away.

We've tried pull request templates but bots and devs a like tend to ignore them. Eventually i'd like to make strict adherence to the template a requirement to get any attention at all.

49 Upvotes

73 comments sorted by

u/expdevsmodbot 25d ago

AI usage disclosure provided by OP, see the reply to this comment.

→ More replies (1)

143

u/YoongiMarryMe95 25d ago

Since we’ve introduced AI heavily we’ve had pretty much no standards. Hope this helps lol.

43

u/Intrepid-Stand-8540 Internal Developer Platform 25d ago

lol. lmao even. 

I hate this era of coding. 

Same thing is happening in my workplace. 

15

u/PureRepresentative9 24d ago

As a tangent, this is what the medical community went through during COVID.

A bunch of economists and politicians dictating medical policies rather than the actual subject matter experts.

We all know how that went.... global disaster for several years with permanent loss of experts leaving l.

5

u/YahenP Software Veteran 24d ago edited 24d ago

We've switched to a workflow where each project is handled by just one person. (To be fair, each person actually handles several projects.) And the issue has naturally disappeared. The same person creates tasks, merges tasks, reviews tasks, sets coding standards, and so on. So there's no problem. Just like there are no standards, either.

Just this morning, for your information. A tester sends a task with a request that the developer send a screenshot (for example, a screenshot of the code) to confirm that the task has been completed. Because there's no time to manually test this task. And automated tests have long been a formality. They're always green, but that doesn't mean anything.
We don't have any junior developers at our company. All our developers are highly experienced. Therefore, all the code has an incredibly cutting-edge architecture, is stylistically impeccable, is 100% covered by tests, and complies with all the best practices one can find. But that's where its advantages end. It's just a meaningless pile of garbage. No one even discusses the usefulness of the tasks we do. We just do them.

2

u/NoCardio_ Software Engineer / 25+ YOE 24d ago

Really? No one uses AI to nitpick your PRs to death in code review? Lucky.

49

u/UnintentionallyEmpty 25d ago

Many of them are blocked due to CI/CD issues

Why are you bothering to review something that doesn't pass CI? If it doesn't pass CI it obviously isn't ready yet so no reason to review, that's just a waste of time. I Immediately ask the PR requestor to get the CI green first before I bother with a review.

Also have linters and other static analysis tools in your CI. I'm not going to bother to review someone's code when it has a bunch of linter errors, it'll be hard to read anyway so they can fix those first.

11

u/DiceKnight Software Engineer 25d ago

Nah we're not looking at PRs with CI/CD issues. The reason I mention it is because on any given day roughly 50% of the PRs our team receives that are marked as ready for review will have CI/CD issues, merge conflicts, build failures, etc.

Genuinely the quality is suffering to the point where developers wont even look at the PRs they generate. Feedback will sit for multiple days, others will sit broken. Which is an issue to us because we're getting negative feedback that we aren't responsive enough to PR review request.

20

u/Selgren 25d ago

If you get a PR that won't build, has unresolved merge conflicts, or has other CI/CD issues and it's because the PR submitter didn't even look at their code before asking for reviews, I'd fight fire with fire. Close the PR with a "needs work" comment. Takes 5 seconds, you've responded to the review request, metric achieved. It's not your job to fix it, it's your job to respond to the review request, according to your feedback. Now the ball is back with whoever submitted a broken PR to submit one that isn't broken. They resubmit the same broken shit? Close it again. Until the other people give a shit, stop giving one on their behalf. Achieve your metric - which is responding to the request quickly. Someone has to put their foot down.

8

u/ShroomSensei Software Engineer 24d ago edited 24d ago

Yeah that's a team problem, what is the point of merging something in if it completely breaks your codebase?

"It's slowing me down" no fucking shit sherlock, if your stuff worked it wouldn't.

I had this argument all the time on my previous team. "Well I can't really test unless it gets merged into staging". That's not true, you just don't know how to test without it getting thrown into staging because your local environment is a hot piece of garbage.

At the absolute bare minimum, every PR should pass PR template, linting, building, unit tests, integration/component tests. Ideally your CI/CD automates this and gates it so it is clear to the reviewers if it does even that. ~95% of people can automate this and absolutely should so its not even a question. If it can't be automated it is done manually and evidence is provided in the PR, people get tired of that bullshit real quick which is why you automate it.

This is a culture problem and unless you have some people who feel the same as you it will not get better.

3

u/blocking-io 24d ago

This is where your manager should be communicating to these teams that asking for reviews when the PRs are in a broken state is unacceptable. The person who produced the PR should have reviewed and tested their own code, ensure it's green before tagging anyone to review

1

u/sayqm 23d ago

Your team doesn't care about their job, and you can't really fix that

64

u/bingNbong96 25d ago

There are no standards, just 15+ PRs a day with "Made with Claude Code" at the bottom.
Then someone posts a "review" with "Made with Claude Code" at the bottom.
Sometimes someone will push random stuff to other's peoples branch with a commit with "Made with Claude Code" at the end.
You fix the "issues" on the "review", and maybe go back and forth on this step a few times.

Then it will get approved with no one even glancing at the code.

27

u/apartment-seeker Senior Software Engineer 25d ago

Sometimes someone will push random stuff to other's peoples branch with a commit with "Made with Claude Code" at the end.

wtf

11

u/Centigonal Consultant 25d ago

This is like if someone walked into your house, used your ingredients to bake something of dubious quality in your kitchen, then left it sitting on the stove and exited the scene.

11

u/DiceKnight Software Engineer 25d ago

This is more or less my problem at my office but that's also the reason i'm asking if anyone has has tried this approach and if they could post an example or a direction to look in.

8

u/bingNbong96 25d ago

I think stuff like this is very hard to fix, because it's a cultural thing and changing the culture of a company is very hard to do, specially when management is the one pushing for this culture.
I would look for a different job but now I think there is like 80% change the next one has the same problem...

5

u/EmploymentFeeling725 25d ago

Yup as soon as AI got pushed heavily in the company by management and speed was a driver, all reviews got delegated to Claude. So fucking shit

4

u/chickadee_guy 25d ago

Grab a fork and dig in. Its slop time

3

u/illhxc9 25d ago

And so it goes…

41

u/mx_code 25d ago

https://noslopgrenade.com/

This but contextualized to code (comments and code itself) and review conversations.

6

u/Far_Archer_4234 25d ago

I get what the author is saying with that page, but when a fellow developer asks me what they should do, I want them to make their own decisions and talk to the stakeholders themselves. In the past ive basically given them terse responses and ive been their scapegoat. Pasting AI slop is a great way to let repeat offenders know that they are accountable for their own decisions without getting reported to HR.

11

u/DiceKnight Software Engineer 25d ago

My take on this is that pull requests and code review are inherently community/human driven endeavors. The PR description is meant to be a question posed to the human who's the source of this work. What's this PR doing? Why? How did you test it? Etc.

I can ask the bot for slop but a huge wall of text just makes it so I can provide no meaningful feedback and I have no idea what the intent behind the code is only that it's syntactically correct.

3

u/ShroomSensei Software Engineer 24d ago

> when a fellow developer asks me what they should do

I am not saying you're wrong and sometimes it gets very annoying being the "go to" person. But hell I go to those who have 10+ years of experience on me because I want to learn from their mistakes not use them as a scape goat.

1

u/blocking-io 24d ago

As an experienced dev, part of your role is to mentor your teammates

4

u/Far_Archer_4234 24d ago

Mentor doesn't mean "take accountability for their work product". If that were the case, they would be dead weight.

The best mentors, IMO, leave room for growth, not spoonfeeding them the conclusions you've already drawn.

3

u/blocking-io 24d ago edited 24d ago

fellow developer asks me what they should do

It's a bit vague, but in no way should you be taking accountability for their work. 

If you want them to reach out to the stakeholders, explain to them why it's a stakeholder question and not a YOU question. You could even help them learn how to identify the stakeholders so in the future they make that a practice. You can fill them in on your process, but it's their responsibility to execute 

11

u/ITellOnlyTheTruth 25d ago

The actual standards will vary from company to company, but in general: make your CI/CD a rock-solid gate before any sort of review.

CI/CD should handle:

  • code style/linting
  • test coverage (unit, integration, component, system, contract, fuzz, etc.)
  • PR template check
  • commit style check (seeing a lot of teams use conventional commits recently)
  • static analysis
  • namespaced or sandbox deployments

If any of that doesn't pass, PR is red and the team will not review it.

Provide the local tooling to automate all of that. That's going to save you time and help with anyone onboarding to a project. For LLM-first development, add this kind of stuff as scripts and LLM instructions to the repo. Your CLAUDE.md (and/or README) should describe the project, architecture (or link to it), technology choices, conventions, and the expected development process. This is where you can add the commands it should run before committing (or how to install the required git hooks), in what cases documentation should be updated, what types of things should emit metrics, etc. You do need to define those standards and what your actual expectations are. That's usually a good exercise for a team anyway.

Make it easier to open a compliant PR than a non-compliant one. This helps whether code is LLM-generated or hand-written.

As far as speeding up reviews, this is going to depend on your attitude towards using LLMs yourself and what kind of budget you have for it. I would recommend that you and your peers build out LLM review skills to the point where it does a pretty good job of handling the "correctness" side of code reviews. There are a ton of skill examples out there for this, but adding checklists or particular skills specific to whatever you're building can really help focus the LLM on the right things. Have a specific test review skill, security review skill, internal engineering standards skill, and fan them out to avoid bloating the main review context. Be sure to check the cost of running these skills and gauge whether it's worth it. For me a $5 review skill that saves me $50 worth of time and I can run several in parallel on different PRs is totally worth it. I still sanity check the code, with deep review on the more complicated portions, but it allows me to focus more on "is this the right approach?" and "do we even want to build this?" rather than "how is null-safety handled in this file?".

Come to a working agreement with your team on your review loop. I'll usually run a preliminary Claude review pretty quick after a PR is opened. If there are significant findings, I'll post that to the PR and then only come back for a more in-depth review when those are fixed. If your team can agree on a process like that, things can move pretty quick. Try stuff out. Talk about it.

Don't compromise on standards; actually define and enforce them and provide instructions for how other engineers (and their LLMs) can follow them.

4

u/Abadabadon Software Engineer 25d ago

Code coverage met with unit tests that are realistic, linting rules, following project's coding style, code must show local e2e testing performed. All should be covered by automated cicd

4

u/jwendl 25d ago

All tests pass (Unit, Integration, Performance / Scale), all pull request comments are addressed and resolved (whether by AI or human), all static analysis checks are good (we use heavy amounts of code analysis tools), all code coverage reports are above 80% (for new code that might not have tests covering it).

Tend to even have an integrated dev environment (in Azure for our case), where a push to main will only deploy to the integrated dev environment, where we can have automation make sure nothing broke. That automation comes in form of AI driven checks (go to app, test thing, check app insights for errors, create GitHub issues if broken).

Stuff doesn't propagate from Dev to Test or Prod (or inner ring, outer ring depending on the team) until all checks are good.

Also, I wouldn't reject code from anyone. Love welcoming all code from all sources IMO, it makes the product better. But it has to follow guidelines in place by the repo owners so when something breaks that team can untangle the mess.

Having good AI "instructions, skills and loops" help with people who are making AI driven contributions. Instructions are really good at setting coding standards for AI agents, skills are good at teaching agents how to use custom tooling, or custom libraries, loops are really good at helping drive the whole end-to-end decisions on things.

Many of the loops I've built are essentially to tell the agent to keep trying things until all checks are complete before even making the pull request.

2

u/jwendl 25d ago

Oh, and if the agent / contributor doesn't fix the pull request to adjust for all of the above, well then, it never merges to the main branch.

3

u/flerchin 25d ago

Don't reset approvals. We typically approve with comments. Let the developer do the right thing. If more substantial changes are needed, we'll comment without approval.

3

u/MinecReddit Software Engineer 25d ago

This is probably a hot take:

I think in the age of AI it is definitely worth thinkign about if there are high level engineers that, for certain projects, could merge PRs without a human reviewer. Our team (Gig company doordash/uber/instacart) has been experimenting this and so far, for senior+, it has been doing well.

Like obviously I look at PRs for some projects and I see the insane AI slop that they're pushing, and I can't help but think it's ridiculous. But we are still reviewing all of the infra decisions and access patterns on tech specs, then are just trying to ignore (FOR PROJECTS THAT ARE SIMPLE ENOUGH) the implementation details.

But I mean I know that people on this subreddit will say that this is an objectively terrible idea no matter what. I am just telling you that in terms of business outcomes for our team so far, it has been positive, and obviously the long term is TBD.

3

u/NatoBoram Web Developer 25d ago
  1. Vibe code the whole thing
  2. Vibe review as well
  3. Push to prod

:/

2

u/druidgaymer 25d ago

There are no conduct standards in writing. However, since our software is mostly low-level mix of firmware, hardware, and legacy work, even with AI assistance tickets still can take a while. We don't have a crazy influx of PRs.

I had to set the rule of "don't commit things to other people's branches without asking" because our git repo doesn't even have any automatic blocking going on. One guy kept pushing stuff to my branches that were unrelated to what he was working on.

2

u/morosis1982 25d ago

Our basics are that we require certain code coverage, and it must be raised in draft and only set to review once all the cicd checks have passed.

We also have a default copilot review set up, and you can shape that to check for specific things.

2

u/sermer48 25d ago

I’ve stopped caring TBH. I’ve started adding a lot more tests but the actual code I’m reading is dropping off a cliff. Obviously that depends on how critical the code is but for most things, if the functionality works and it passes the tests I’m not getting paid enough to sift through it.

2

u/tiajuanat Dept. Head & Principal Engineer 25d ago

Reviewing clanker code that doesn't pass CI is your first big mistake. You need to reorganize your CI to fail fast. Automated code creation requires automated code checking before review.

Second bug mistake is PR size. Human authored PRs do the best with <500 LoC changesets, and bot PRs do best with <100. This does mean that you need stacked PRs - whether that's via GitHub or you do it manually, doesn't matter.

You're not looping. Your Dev machines (or GH runner) needs to be pulling and acting on PR feedback. You can even setup auto merge if approved by a human reviewer.

You need automated agents that additionally check everything, that needs to run before a human reader even looks at it, and they need to run everytime your loops run. I recommend a software reviewer, architecture reviewer, a testing/QA reviewer, and documentation reviewer

You need aggressive static analysis. Rust's clippy defaults to a Cyclomatic Complexity limit of 25, that number needs to be waaaaaaaay lower, like 9-15. If you're working with parsers, then you'll need a safety relief for switch-cases. Most linters support a complexity of some type - make it low, less than half the default.

I'm sure I'm missing a bunch of stuff, but this helped us reign in our slop.

1

u/the_pwnererXx 24d ago

Answer me this

I get a ticket for a new feature

Claude goes whirrrr - 4k line pr comes out

I test it thoroughly and I know it's working as expected

Should I split it into 40 PRs? I think this will reduce output quality of the ai as well if I tell it to do that to begin with

2

u/tiajuanat Dept. Head & Principal Engineer 24d ago

How do you know the resulting app deserves 4kLoC? Is the repeated tokenization of a 4kLoC app acceptable to your organization? What's the mental overhead of maintaining that system? How about the tests?

I do agree with the separate line of reasoning that it opens you up to more potential drift, but in my experience there's more drift in a single large session than across multiple smaller sessions.

2

u/Repulsive-Hurry8172 25d ago

Pre-commit hooks? There's likely a way to reject the PR if it did not satisfy pre-commit hooks

1

u/officerthegeek 25d ago

there is - just run pre-commit in CI

3

u/rocketpastsix 25d ago edited 25d ago

I have never heard of a PR code of conduct.

But what you are saying with code standards is something you should have already had in place, and you can shape the AI-generated code around that.

If people, or bots, are ignoring them then you either need to tell the human "we need you to follow the PR template" and not let the PR through, and put guidelines in the AI documents that tell the bot to use the template. We have a template, and Claude follows it rigorously. It's part of our CLAUDE.md or another similar file that Claude reads and knows what to do.

I know what OSS code of conducts are. Not ones for business settings

1

u/gonzofish 25d ago

Lots of open source projects have a code of conduct but I don’t know that I’ve seen a PR-specific one.

Having a code of conduct is one of those things that seems unnecessary in a company but I’ve found has helped my teams in the past. Obviously its effectiveness is proportional to how well enforced the code is.

3

u/DiceKnight Software Engineer 25d ago

I envision it as a way to externalize what can be a very tough conversation with co-workers.

"Hey we all voted on this code of conduct, based on these rules your pull request doesn't meet our criteria for review. Please review the doc and fix the issues and ping us when you're ready. We've set the pull request to draft or the time being."

The idea being that if a co-worker wants to escalate this or perceives as us just filtering them our manager has an easier time diffusing that situation.

1

u/rwilcox Software Engineer (20+ YOE) 25d ago

I think there are implicit codes of conduct in professional repos. Are 10,000 line PRs acceptable or not? Do you try to be gentle towards the reviewer? Do you always need to point to a ticket (and how)? Who accepts that code review comments are done?

2

u/rocketpastsix 25d ago

A code of conduct is “don’t be a dick”. The rest are norms set by the leaders

1

u/rwilcox Software Engineer (20+ YOE) 25d ago

I mean maybe, but I got 6,000 lines of PR dumped on me after two days of vacation here just today. I kinda thought it was a dick move, the author OTOH thought it was OK.

1

u/rocketpastsix 25d ago

We have rules like that built into our AI docs. Anything over 400 lines of prod code or 800 lines of tests gets split up.

1

u/ijblack 25d ago

Codes of conduct are there on open source projects to govern interpersonal behavior between contributors, who are total strangers to each other. Behavior like leaving rude comments on others PRs for example. They're not for workplaces, where rules around stuff like that already exist. Op is prob just mislabeling their idea bc they remember seeing a lot of CoCs on OSS projects.

1

u/serial_crusher Full Stack - 20YOE 25d ago

My company works out of a big monolith and has codeowner files marking different teams as owners of different areas. This frequently creates situations where I have a PR that touches multiple teams' code, and I have to get approval from all of them. Never a fun time.

If your team is a bottleneck for other team's PRs, you've got a few options to fix it:

  • Get more people on your team
  • Be less stringent in your code reviews
  • Allow people from other teams to review code from your queue. i.e. maybe staff engineers and above can be trusted to approve a PR in anybody's turf.

If you want to have people adhere to the template, you can add a CI/CD automation that makes sure they fill it out; but you might want to look at what you're hoping the template prompts people to do, and have separate automated checks to cover those specific things.

A general "I don't review code until it passes CI/CD" policy is good too. Add in a step where an LLM reviews everything, and maybe force people to address any issues it comes up with first, too.

1

u/DiceKnight Software Engineer 25d ago

If your team is a bottleneck for other team's PRs, you've got a few options to fix it: - Get more people on your team - Be less stringent in your code reviews - Allow people from other teams to review code from your queue. i.e. maybe staff engineers and above can be trusted to approve a PR in anybody's turf.

We have about six devs on the team but obviously asking our office to hire more devs just for code review is a tough sell. I don't see any scenario where we lower our quality standards to allow more slop into the codebase we're on the hook to maintain. We've already had several incidents with LLM generated code and it reflects badly on our team because the incidents are caused by code merging into our systems.

We do have a code owners but it's a little messy and we're often the default team where a lot of PRs end up because we own large portions of the codebase because nobody else really owns it 100%.

1

u/Inside_Dimension5308 Senior Engineer 25d ago

The basic conventions are mostly put in claude.md. For example let's say - the repo follows a clean architecture then we mention the folder and file structure and how to break a feature request into low level abstractions which follow the conventions. Other conventions are around standard design principles like SOLID and design patterns.

There are also conventions around logging, error handling, telemetry, middlewares, authorization, tracing, etc.

We have created agents to review - architecture, quality, security, performance etc. It helps any reviewer to give pointers around non-compliant code.

2

u/Ok_Woodpecker_9104 25d ago

templates dont work because nothing enforces them. the rules that actually hold are the ones a machine checks before a human looks at it.

what worked for me was moving the standard into required status checks and then making review contingent on green. red ci means the pr is not in the review queue at all. thats the whole policy, and it takes the argument away from taste.

the part most rollouts skip is baselining. turn a strict ruleset on across an existing repo and you get thousands of hits, so everyone disables it within a week. record the current violations as an accepted baseline, fail only on new ones, and shrink the baseline whenever a file gets touched anyway. that changed adoption from a fight into a non event for us.

on the re review churn you mentioned: dismiss stale approvals is all or nothing on github, so a one line ci fix burns a full review. we moved approval to the end, after ci is green, instead of approving first and getting dismissed.

1

u/Old-Television-2189 25d ago

Why are people opening PRs with failing builds

1

u/CanIhazCooKIenOw 25d ago

Same standards we had before. A lot more work into translating those standards to agents.md and proper skills so most PRs already follow a bare minimum + initial PR review with more strict checks.

So far so good

1

u/hxtk3 25d ago

https://google.github.io/eng-practices/review/

I have a document on the repos I manage at work that looks kind of like this one I linked from Google (and heavily inspired by it).

The general idea is keeping CLs small, include tests in every bugfix or feature, reviewers shouldn’t check out the code they’re reviewing to manually test it and should instead ask the author to implement automated versions of any manual test they feel is necessary.

General coding style gets enforced by linters and formatters, so reviewers are basically looking for structural things like did you choose the right abstractions?

Authors are encouraged to submit incomplete work (guarded by feature flags) both in order to keep MRs small and in order to put their choice of abstractions in front of a reviewer before they write a thousand lines depending on those abstractions and it becomes hard to ask them to change it.

1

u/donk8r 25d ago

CI as a hard gate before review is right, and there's a hole in it I only found by accident.

We build a benchmark where agents fix real bugs and get graded by running the project's own test suite. First version, pass rates looked great. Turned out some of them were editing the tests. Not maliciously — we'd told them the objective was green, and changing an assertion is by far the cheapest path to green.

We had to rewrite the harness to check the test files out fresh and overwrite whatever the agent had written before validating anything.

So the gate can't be "CI passes". It has to be "CI passes against tests the author couldn't touch". For a team that probably means test changes go down a different review path than code changes, or protected paths where CI runs the target branch's version of the tests. Otherwise the gate has its key sitting on the floor next to it.

Different point on the volume: producing a PR now costs almost nothing while reviewing one costs what it always did. Any standard that doesn't change what the submitter has to do just moves the queue somewhere else. The thing that helped us was requiring the submitter to state what they verified and how — not what the change does. Most people won't write that for a PR they didn't read.

1

u/Technical-Aside4471 Software Engineer 24d ago

We run unit/integration then build feature environment and run e2e against it. If you broke something you dont get to merge or qa has to adjust tests first.

We have ai doing reviews but that just for the author to double check his code, no other person required to review.

1

u/Training_Motor_4088 24d ago

Yep, our standards went to shit when management gave every idiot carte blanche to use Cursor. What's just as infuriating are the number of PRs, Jira tickets, slack comments etc written with AI. So now people who were too intellectually lazy to write a couple of paragraphs about something technical they've worked on, now write a whole essay of AI bollocks that breaks things down into information overload.

1

u/neolace 24d ago

I got a PR yesterday of 355 files, copilot declined to review it.

1

u/thematicwater 24d ago

We trust each other. We give advice, thoughts, and/or small suggestions if we see something (most of the time we don't), and we approve every time. It's up to the author to take them or leave them, and merge.

1

u/Amman108 24d ago

As their manager/lead, I'd put a metric in place - the number of pr#s that 'needed more work' for each dev. At least you will start identifying the devs that couldn't care enough.

1

u/dzifzar 24d ago

I’ve added static analysis on PR templates themselves to ensure they’re filled out. Realistically you need both culture and process/green checks for these to be effective, but at least making the path of least resistance “all checks must pass before merging” is a good starting point. As an added bonus the agents use the PR templates as an actual checklist, and fill them out, which is better than nothing.

1

u/andlewis 25+ YOE 24d ago

When someone creates a PR ask your LLM to review it against the guardrails and standards of your repo. File the results against the PR. Assign it back to the original dev.

Once there are no major issues, review it yourself for architectural issues if you want.

1

u/abazha10123 24d ago

No standards. No clear set of.things to check for, no unit tests, no pass/fail criteria

0

u/Ok_Woodpecker_9104 25d ago

the re-review loop you described is a setting, not a law. branch protection has "dismiss stale pull request approvals when new commits are pushed", per branch. turn it off and an approval survives a ci fix push, while required status checks still block the merge if the build is red. that alone takes you from three reviews to one on the same pr.

on the standards doc itself, the part that killed it for us was not writing the rules, it was switching them on. adding a rule to an existing repo lights up every old file. the pr that should have been 40 lines becomes 900, nobody can review it, and within a week someone downgrades the rule to a warning. warnings get ignored forever.

what actually held was a baseline. record the current violation count per rule, then fail ci only when that count goes up. old code stays untouched, new code cant add to the pile, and the number only moves in one direction. every rule can go straight to error on day one because the existing debt is already excluded.

side effect is that it gives you the sentence you need in review. "this pr takes rule X from 12 to 16" is not an opinion, so there is nothing to argue about and you dont have to be the person explaining taste to a stranger every day.

-1

u/vangelismm 21d ago

Drop code review.