index_together should also have the same convinience as unique_together

181 views
Skip to first unread message

anubhav joshi

unread,
Mar 1, 2014, 3:50:22 AM3/1/14
to django-d...@googlegroups.com
"For convenience, unique_together can be a single tuple when dealing with a single set of fields"

This convinience is missing in index_together:
I have opened a PR, please see: https://github.com/django/django/pull/2381

Regards,
Anubhav Joshi
IIT Patna

anubhav joshi

unread,
Mar 1, 2014, 3:50:51 AM3/1/14
to django-d...@googlegroups.com

Florian Apolloner

unread,
Mar 1, 2014, 6:46:33 AM3/1/14
to django-d...@googlegroups.com
Hi,

there is no need to mail django-developers when you open a PR -- we do monitor the relevant lists.

Cheers,
Florian

Christian Schmitt

unread,
Mar 1, 2014, 7:16:45 AM3/1/14
to django-d...@googlegroups.com

Hi,
Currently It doesn’t look so.
I mean some pull requests are open since two years. Why don’t you close them, when they won’t apply anymore?
It looks like no Core Developer is willing to clean them, etc…
I mean a lot of new features are coming, but it doesn’t look like old things getting cleaned up and updated.

Best Regards

Christian Schmitt




--
You received this message because you are subscribed to the Google Groups "Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/68226a0a-d08b-4b52-b8a1-8d9f80c58e85%40googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

signature.asc

Aymeric Augustin

unread,
Mar 1, 2014, 11:56:13 AM3/1/14
to django-d...@googlegroups.com
On 1 mars 2014, at 13:16, Christian Schmitt <c.sc...@briefdomain.de> wrote:

> I mean some pull requests are open since two years. Why don’t you close them, when they won’t apply anymore?

Because it feels aggressive to the people who submitted the pull requests.

> It looks like no Core Developer is willing to clean them, etc…

I did until I was asked to stop.

--
Aymeric.

anubhav joshi

unread,
Mar 1, 2014, 4:49:46 PM3/1/14
to django-d...@googlegroups.com
This is now fixed in bb2ca9f.
PR : #2381

Josh Smeaton

unread,
Mar 1, 2014, 8:40:41 PM3/1/14
to django-d...@googlegroups.com


On Sunday, 2 March 2014 03:56:13 UTC+11, Aymeric Augustin wrote:
On 1 mars 2014, at 13:16, Christian Schmitt <c.sc...@briefdomain.de> wrote:

> I mean some pull requests are open since two years. Why don’t you close them, when they won’t apply anymore?

Because it feels aggressive to the people who submitted the pull requests.

Isn't this preferable to seeming indifferent? I just opened a random selection of PRs and about half had recent comments from core devs (or old comments from core devs that were never replied to). The other half were a mixture of "ping" and no reviews or comments. While I understand that core devs are a limited resource, I feel a long PR queue could potentially scare off new contributors. 
 

> It looks like no Core Developer is willing to clean them, etc…

I did until I was asked to stop.

 
Maybe it'd be worth re-evaluating the situation? At the very least it might be a good middle ground to tag PRs with "Requires Work" or "Not Ready", so they can be easily filtered out from "active" PRs. On the other hand, there could be a strange mix of Trac and the PR queue containing conflicting information.

Just tossing out some thoughts.

Regards,
 

Aymeric Augustin

unread,
Mar 2, 2014, 3:15:43 AM3/2/14
to django-d...@googlegroups.com
On 2 mars 2014, at 02:40, Josh Smeaton <josh.s...@gmail.com> wrote:

> At the very least it might be a good middle ground to tag PRs with "Requires Work" or "Not Ready", so they can be easily filtered out from "active" PRs. On the other hand, there could be a strange mix of Trac and the PR queue containing conflicting information.


In short, Django uses PRs for code review, and nothing else. The status of a PR is defined by the status of the Trac ticket that contains a link to that PR. The ticket lifecycle is explained in the contributing guide.

GitHub's issues / PR system is designed for small projects. It's improductive and impractical to look at the list of pull requests and work from there. So we don’t do that. If you need to filter tickets, do it in Trac.

In detail, we cannot make a better use of PRs because GitHub’s UX is pathetic as soon as you have more than two dozen PRs. Having brought the number of issues on the Django Debug Toolbar down from 150 to 20, I have first hand experience of banging my head against GitHub Issues on a projet much smaller than Django, but already too large for GitHub. It’s simply impossible to navigate issues efficiently when all you have is binary open / close (and contributors can’t reopen when you close), tags (if you emulate state with tags, some tickets end up with multiple states and others with no state), pagination hardcoded at 30 per page, and that’s it. Also, if I remember correctly, we cannot receive notifications on the django-updates mailing list when someone makes a pull request, which doesn’t help.

I wish I could say “sure, we’ll give you access, show us how it would work” but that would mean giving you commit access to Django. At that point, we’re hitting the other big limitation of GitHub: if you aren’t a committer, you cannot do anything for a projet. But Django relies a lot on its broader community. So, once again, if you’d like to clarify the status of issues, the place to do it is Trac. (Triagers get a few extra permissions, just ask if you want them.)

I hope this helps you understand our choices better. I wish we could do everything on GitHub, but our body of knowledge is in Trac and its features simply blow GitHub out of the water.

--
Aymeric.

Josh Smeaton

unread,
Mar 2, 2014, 3:59:48 AM3/2/14
to django-d...@googlegroups.com
Good points all.

Looking more closely at Trac, the combinations of "has patch" "patch needs improvement" and "ready for checkin" fields should cover the status of PRs.

I'll try to set some time aside to go through the PRs and check that the status in Trac matches. Though thinking about it, going through 100-odd PRs and checking if they apply cleanly seems like a daunting task. Is there anyway of indicating that a patch needs a review? Or is simply setting "ready for checkin" a good enough signal?

Cheers

Aymeric Augustin

unread,
Mar 2, 2014, 4:04:47 AM3/2/14
to django-d...@googlegroups.com
On 2 mars 2014, at 09:59, Josh Smeaton <josh.s...@gmail.com> wrote:

> Is there anyway of indicating that a patch needs a review? Or is simply setting "ready for checkin" a good enough signal?

"Ready for checkin” means that you have reviewed the patch and you think it’s complete and correct.

If the patch applies cleanly but you haven’t looked at it, the correct status is “Accepted” + “has patch”.
That combination implies that the patch needs a review.

--
Aymeric.

Reply all
Reply to author
Forward
0 new messages