2 ms·
TL;DR: - Don't worry about it! - Resolve an existing issue - Make your pull request in a way that makes it pleasant to review by including prose and screensho
by gknoy 13y ago
TL;DR:
- Don't worry about it!
- Resolve an existing issue
- Make your pull request in a way that makes it pleasant to review by including prose and screenshots (if applicable)
- Expect please-fix-this comments.
Pull requests are a vehicle for you to help improve a codebase, but also for the maintainers to evaluate whether your contribution is a good fit for their project. Rejection hurts. However, often you can aim for criticism that says, "I'll merge this if you fix X, Y, and Z".
Treat a PR as a request for feedback on your code. Perhaps the project prefers a certain style, or the maintainers might know of ways you can write your code in a more idiomatic manner for that language. Don't be afraid of this! This is one of the ways we get better. Expect to have several comments that are "please-fix" style comments.
Start small, and try to resolve an issue that's already reported. Ask a maintainer if there's a small enough task that they'd be willing to delegate to you. If they already know you're working on something, you are likely to get a warmer reception, and you avoid them having the same feature already under development.
When you finish implementing your feature (or fix), spend some time writing a few paragraphs of prose describing what your PR changes, what problems it fixes, as well as any assumptions you made or limitations that it might have and any future changes that this might need.
e.g.:
This changes the way we fetch Foo and Bar lists
of items for users so that they use the same RPC
method. In a future change, I plan to merge the
separate tables into a single display which can
be filtered in a more robust way.
This helps give a gentler introduction to your change, and lets you explain why you feel it's necessary. It also lets you describe the issue you're resolving (In addition to saying "Fixes #1324") so that it's easier to read. If you're changing the way things are displayed, try to include a screenshot.
- japhyr 13y agoDoes it make sense to get in touch with developers first, to find out where they'd like you to focus, or to find out if there is already ongoing work on an issue?