Skip to content

Conversation

@sobolk
Copy link
Contributor

@sobolk sobolk commented May 19, 2023

Issue #, if available:

Description of changes:

Something more realistic.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.


const MAX_LINES_ADDED = 200;
const MAX_LINES_REMOVED = 200;
const MAX_LINES_ADDED = 1000;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The goal I'd like to aim for here is 30 min reviews (or less). I'm open to bumping this up but worried it will only ever go up. Another knob we could tweak before raising this too much is to exclude some config files. This won't help in all cases, but in cases where we are adding a new dev tool or a new package this will prevent some of the config boilerplate from counting towards the line limit

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've extrapolated these numbers from PRs we already had and nobody complained about size. All were reviewable in <10 minutes for me at least.

I do agree that we should not keep bumping that. But I think we started to strict.

@sobolk sobolk merged commit da8c466 into main May 20, 2023
@sobolk sobolk deleted the sobolk-patch-1 branch May 20, 2023 00:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants