Skip to content

CONTRIBUTING.md: Fill out section on pull requests.#6879

Merged
cwfitzgerald merged 1 commit intogfx-rs:trunkfrom
jimblandy:contributing-pull-requests
Feb 24, 2025
Merged

CONTRIBUTING.md: Fill out section on pull requests.#6879
cwfitzgerald merged 1 commit intogfx-rs:trunkfrom
jimblandy:contributing-pull-requests

Conversation

@jimblandy
Copy link
Member

No description provided.

@jimblandy jimblandy requested a review from a team as a code owner January 8, 2025 19:17
@ErichDonGubler
Copy link
Member

I think I'm going to be able to continue reviewing this on Tuesday of this coming week. 🫡 See you then!

Copy link
Member

@ErichDonGubler ErichDonGubler left a comment

Choose a reason for hiding this comment

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

I think this new content is good, but its relationship with other content needs some work. Let's handle the current conversations, and then see where we're at.

@jimblandy jimblandy force-pushed the contributing-pull-requests branch 2 times, most recently from 23c20a2 to 788ac36 Compare January 30, 2025 18:10
@jimblandy
Copy link
Member Author

Hmm, the large PR language still isn't quite right.

@jimblandy jimblandy closed this Jan 30, 2025
@jimblandy jimblandy reopened this Jan 30, 2025
@jimblandy jimblandy marked this pull request as draft January 30, 2025 18:15
@Vecvec
Copy link
Contributor

Vecvec commented Jan 31, 2025

I think the wording is much better, although I'm not a WGPU maintainer (and so don't have as much experience with PRs) it seems to match with what my experience of code being reviewed feels like.

Copy link
Member

@ErichDonGubler ErichDonGubler left a comment

Choose a reason for hiding this comment

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

Love the explanation of historical trauma and elaborating why large PRs can wrest control from maintainers in a way they can't tolerate.

Otherwise, only a couple of nits.

Approving, trusting that @jimblandy will fix up the non-non-blocking nits before merging.

Copy link
Member

@cwfitzgerald cwfitzgerald left a comment

Choose a reason for hiding this comment

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

Approving with similar nit

@cwfitzgerald cwfitzgerald marked this pull request as ready for review February 12, 2025 18:24
@jimblandy jimblandy force-pushed the contributing-pull-requests branch 3 times, most recently from c74ffe2 to cca6022 Compare February 24, 2025 19:11
@jimblandy jimblandy force-pushed the contributing-pull-requests branch from cca6022 to 2ddebc3 Compare February 24, 2025 19:12
@jimblandy
Copy link
Member Author

Re-requesting review from Vecvec and Erich since I mostly rewrote the section they commented on.

Copy link
Contributor

@Vecvec Vecvec left a comment

Choose a reason for hiding this comment

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

I think the new version has fixed the issues I had with this. The additions to the PR template are nice.

Copy link
Member

@cwfitzgerald cwfitzgerald left a comment

Choose a reason for hiding this comment

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

Love it!

@cwfitzgerald cwfitzgerald merged commit 7cbad8e into gfx-rs:trunk Feb 24, 2025
34 checks passed
sharmajai pushed a commit to sharmajai/wgpu that referenced this pull request Oct 12, 2025
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.

4 participants