Updating contributing documentation

[Riccardo Magliocchetti]

Hello,

I'd like to update the contributing (docs/intro/contributing.txt) docs
to make the process a bit smoother, here's some issues i've found:

- from my own experience it looks like that a pull request is required
for review. Documentation say patches in the bugtracker are fine but
have been asked to open pull request even with patches sitting in the
ticket.

- there's no mention on the ready for checkin bit, again from my own
experience there has been patches in the bugtracker for years because
without that bit sets the patch is off the radar of reviewers

- suggest to run pyflake8 on the file you changed, saves a review round trip

- there should be some switch that makes deprecations warning fail on
djangoci but i don't see them when running tests on my machine. That
could easily done on developers machines to save a round trip while
reviewing. Any hint please?

If there's agreement with this i can open a pull request.

Thanks,
riccardo

[Riccardo Magliocchetti]

Il 04/02/2015 10:15, Riccardo Magliocchetti ha scritto:
> Hello,
>
> I'd like to update the contributing (docs/intro/contributing.txt) docs
> to make the process a bit smoother, here's some issues i've found:
>
> - from my own experience it looks like that a pull request is required
> for review. Documentation say patches in the bugtracker are fine but
> have been asked to open pull request even with patches sitting in the
> ticket.
>
> - there's no mention on the ready for checkin bit, again from my own
> experience there has been patches in the bugtracker for years because
> without that bit sets the patch is off the radar of reviewers

Here i meant the "patch needs improvement" bit

[Tim Graham]

Hi Riccardo,

Thanks for the feedback. Maybe adding a link to the patch review checklist [1] to the contributing tutorial would be sufficient. Most of the points you've covered are valid and are covered in the rest of the contributing docs. I'd like not to duplicate information if possible, but please do submit a pull request with changes you feel are appropriate.

[1] https://docs.djangoproject.com/en/dev/internals/contributing/writing-code/submitting-patches/#patch-review-checklist

- from my own experience it looks like that a pull request is required
for review. Documentation say patches in the bugtracker are fine but
have been asked to open pull request even with patches sitting in the
ticket.

Yes, pull requests are preferred, especially for any non-trivial code changes. I still use attachments for smaller doc patches. We should likely recommend attachments only to those who cannot submit a pull request.

- there's no mention on the ready "Patch needs improvement" bit, again from my own

experience there has been patches in the bugtracker for years because
without that bit sets the patch is off the radar of reviewers

I've thought to address this issue by improving the UI in Trac so that it translates the current flags to a sentence that summarizes the "next steps" for the ticket.
https://github.com/django/code.djangoproject.com/issues/33

- suggest to run pyflake8 on the file you changed, saves a review round trip

This is covered in the patch review checklist.

- there should be some switch that makes deprecations warning fail on
djangoci but i don't see them when running tests on my machine. That
could easily done on developers machines to save a round trip while
reviewing. Any hint please?

This is covered in the "Deprecating a Feature" guide, which is linked from the patch review checklist.
https://docs.djangoproject.com/en/dev/internals/contributing/writing-code/submitting-patches/#deprecating-a-feature

[Riccardo Magliocchetti]

Hi Tim,

Il 04/02/2015 13:24, Tim Graham ha scritto:
> Hi Riccardo,
>
> Thanks for the feedback. Maybe adding a link to the patch review
> checklist [1] to the contributing tutorial would be sufficient. Most of
> the points you've covered are valid and are covered in the rest of the
> contributing docs. I'd like not to duplicate information if possible,
> but please do submit a pull request with changes you feel are appropriate.

> [1]
> https://docs.djangoproject.com/en/dev/internals/contributing/writing-code/submitting-patches/#patch-review-checklist

Ah! here's the docs i need :)

Anyway here's a pull request that add a couple of cross references to
possible other interesting pages to the reader:

- from first patch tutorial to submitting patches, in case someone
looking for a patch checklist get here from a search engine
- adding a link to the aforementioned tutorial from the new contributors
page for newbies looking for the basics.

https://github.com/django/django/pull/4050

hth,
riccardo