ably / engineering

Ably's new home for engineering guidance, including content previously served from Confluence at engineering.ably.com.
https://engineering.ably.com/
Apache License 2.0
2 stars 6 forks source link

Pull Request etiquette #16

Closed QuintinWillison closed 2 years ago

QuintinWillison commented 2 years ago

The guidance in this new section is designed to enhance how this document reflects our current ways of working in the SDK Team, so I'm hoping that none of it is surprising or contentious.

Reviewers can preview the rendered markdown here: https://github.com/ably/engineering/blob/pr-etiquette/sdk/pull-requests.md#etiquette

QuintinWillison commented 2 years ago

I've invited other contractors ('outside collaborators' in GitHub nomenclature) to have Write access to this repository so that they can participate in this review. As they accept those invites I shall add them as reviewers.

QuintinWillison commented 2 years ago

@tomkirbygreen I appreciate completely where you're coming from in respect of the size of PRs, and I'm sure you have particular experiences that have influenced this being a particular complaint. However, I'm not going to add anything to this particular PR on that as it's potentially quite a subjective area and will depend on the PR context (e.g. which SDK, what type of work is being done, etc..). We can always write something up on this in future, however my feeling is that generally as long as we all stick to agile principles then PR size should 'look after itself'. cc @jamienewcomb

QuintinWillison commented 2 years ago

Thanks, everyone, for your input on this pull request.

We can and will evolve this in future if our needs or ways of working change, of course. Equally, for those who didn't get an opportunity to contribute to this review, please feel free to create an issue or even a PR. 😁