Comments (9)
If a maintainer has submitted a pull request and it has not received approval from at least one other maintainer, it can be landed after 48 hours
By this I mean to say that if there has been no activity on the PR, including approvals. If there is active discussion but no approval it shouldn't be landed.
from sdk-javascript.
- If a maintainer has submitted a pull request and it has not received approval from at least one other maintainer, it can be landed after 48 hours
This sounds a little short to me, thinking about weekends for example. Perhaps 72 hours?
from sdk-javascript.
Yeah probably 72 hours will fit better.
from sdk-javascript.
@danbev that sounds reasonable to me.
from sdk-javascript.
I'm not sure what the current issue is. I also think we could just use existing tooling instead of having a doc with rules in presumably markdown docs. Examples:
- Required PR status check: https://help.github.com/en/github/administering-a-repository/about-required-status-checks
- Required reviews: https://help.github.com/en/github/administering-a-repository/enabling-required-reviews-for-pull-requests
I don't want to have an SLO that can't be enforced or that isn't a problem currently.
from sdk-javascript.
@grant I agree that the automated checks are good. That's why my first guideline is "No pull request may land without passing all automated checks".
But I think some additional organizational structure is useful, particularly for people who live in different time zones. For example, I believe @danbev is 9 hours ahead of you. If we want to have solid engagement from contributors across the globe, it's important to give consideration to the fact that he might want to be included in the pull/review/land process. But if you submit a PR when you get up in the morning, on Friday and land it on Saturday morning, there's a good chance he will have completely missed all of that.
I actually think that these guidelines are even looser than they should be. My original idea was.
- All maintainers can land pull requests from outside contributors 24 hours after they have been submitted, given that it has one approval (that approval can be from the person landing the PR)
- If a maintainer has submitted a pull request and it has approval from one other maintainer, it can be landed after 24 hours
- If a maintainer has submitted a pull request and it has not received approval from at least one other maintainer, it can be landed after 72 hours
(well tbh, I did not have 72 in my original draft, but taking @danbev's cue)
In my view this isn't about SLO, it's about respecting the community and other maintainers, giving them an opportunity to weigh in on pull requests before they land.
from sdk-javascript.
All maintainers can land pull requests from outside contributors 24 hours after they have been submitted, given that it has one approval (that approval can be from the person landing the PR)
If a maintainer has submitted a pull request and it has approval from one other maintainer, it can be landed after 24 hours
@lance I think we should not have a time minimum for PRs approved from at least one mantainer (other than the author). This slows down hotfixes, like it happened in past in sdk-go where i broke some stuff and we needed an urgent fix to release 😄 If a PR is approved, then it's fine. Of course, it's at discretion of the mantainer to avoid landing a big PR too soon, without proper review
from sdk-javascript.
@slinkydeveloper good points. I was trying to think of situations where a maintainer might want to stop a pull request from landing but because of time zones may not see it immediately. Do you think it's overkill? There could be other ways to handle hot fixes - with labels for example. But maybe we don't need to bother with this unless it becomes an issue.
So how's this?
- No pull request may land without passing all automated checks
- All pull requests require at least one approval from a maintainer before landing, with one exception noted below
- A pull request author may approve their own PR, but will need an additional approval to land it
- If a maintainer has submitted a pull request and it has not received approval from at least one other maintainer, it can be landed after 72 hours
- If a pull request has both approvals and requested changes, it can't be landed until those requested changes are resolved
from sdk-javascript.
I agree with that second version 😄
from sdk-javascript.
Related Issues (20)
- Unnecessary polyfill for http and https required HOT 3
- Harmonize validation of CloudEvents HOT 9
- Avoid jest#13535 and planttheidea/fast-equals#91
- [security] Potential XSS in httpTransport() HOT 3
- Does not work in browsers as of 5.3.1 HOT 1
- Add the engines property to the package.json HOT 1
- Add Tests for the Browser version HOT 2
- No .d.ts files available for browser bundle HOT 1
- Add Test Runner for Node 18 LTS HOT 1
- Can't resolve 'http' error when using on React app HOT 11
- Vulnerability of util version 0.12.5 HOT 2
- Invalid default data for parsing in binary mode HOT 7
- Getting error when installing on node 20.0.0
- Compatible with Node 20.x HOT 1
- Add support for request timeout in HTTP transport HOT 2
- HTTP headers name case for extensions HOT 3
- Unable to retrieve data from the HTTP request HOT 5
- Consider removing or breaking out http/request helpers HOT 3
- Support for AVRO event format HOT 2
- Support for Protobuf event format HOT 2
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from sdk-javascript.