| Message ID | 1471943023-1112-6-git-send-email-wengpingbo@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
On 23 Aug 17:03, WEN Pingbo wrote: > Added a ForeignKey in submission, and linked releated patches by > 'In-Reply-To' or the first 'References'. > > Child emails must fill 'In-Reply-To' or 'References' with correct msg-id, > or the parent field will keep empty. > > Signed-off-by: WEN Pingbo <wengpingbo@gmail.com> I need to think on this a little more, but I don't think will scale very well. The main issues is that it's essentially a linked list, which SQL doesn't handle well, and the Django ORM handles it even worse. Some conditions that I think this would fail on: * The cover letter gets dropped by your mailing list (Mailman?) This will mean you've nothing to set as 'parent' for the series * You're using nested patches (each patch is a reply to the previous patch) and one of those gets lost Same as above * You're using nested patches and your series is rather large This will require N queries, where N is the number of patches in the series The last of these is probably the most serious. Have you seen the series work already submitted to the mailing list [1]? It still needs work, but it's being actively tested and could fit your purposes once merged. [1] https://lists.ozlabs.org/pipermail/patchwork/2016-June/002873.html
> 在 2016年8月30日，07:21，Stephen Finucane <stephenfinucane@hotmail.com> 写道： > > On 23 Aug 17:03, WEN Pingbo wrote: >> Added a ForeignKey in submission, and linked releated patches by >> 'In-Reply-To' or the first 'References'. >> >> Child emails must fill 'In-Reply-To' or 'References' with correct msg-id, >> or the parent field will keep empty. >> >> Signed-off-by: WEN Pingbo <wengpingbo@gmail.com> > > I need to think on this a little more, but I don't think will scale > very well. The main issues is that it's essentially a linked list, > which SQL doesn't handle well, and the Django ORM handles it even > worse. Some conditions that I think this would fail on: > > * The cover letter gets dropped by your mailing list (Mailman?) > This will mean you've nothing to set as 'parent' for the series > * You're using nested patches (each patch is a reply to the previous > patch) and one of those gets lost > Same as above Yes, if one of patch in series get lost, or email client fills the references with empty or wrong msg-id, or a child patch is arrived first than parent, the list will be broken. I think the first of two is not patchwork problem, at least can’t fix in patchwork. And I don’t have much idea to find a parent patch precisely without correct msg-id so far. I had try to fix the last case, by walking through all orphan patch if we find a submission with empty parent. But it’s too expensive, and I don’t see much list recovering in my server. > * You're using nested patches and your series is rather large > This will require N queries, where N is the number of patches in > the series Indeed, we need to spend O(N) time if we get a N depth list. Maybe we can add a limit here. Only show the first ten patches in a series by default, and user need to click ’Show more patches’ link to load the next ten patches in a series. > The last of these is probably the most serious. > > Have you seen the series work already submitted to the mailing list > [1]? It still needs work, but it's being actively tested and could fit > your purposes once merged. > > [1] https://lists.ozlabs.org/pipermail/patchwork/2016-June/002873.html Thanks, I will check those patches later:) Pingbo
diff --git a/htdocs/css/style.css b/htdocs/css/style.css index f7f7b6a..702b7ed 100644 --- a/htdocs/css/style.css +++ b/htdocs/css/style.css @@ -336,9 +336,9 @@ div.patchform h3 { } div.patchform ul { - list-style-type: none; padding-left: 0.2em; - margin-top: 0em; + margin-top: 0.5em; + margin-left: 1.5em; } /* forms */ diff --git a/patchwork/bin/parsemail.py b/patchwork/bin/parsemail.py index 6f644da..bcc7982 100755 --- a/patchwork/bin/parsemail.py +++ b/patchwork/bin/parsemail.py @@ -318,6 +318,16 @@ def find_submission_for_comment(project, refs): return None +def find_submission_for_ppatch(project, ref): + submission = None + + if not ref == None: + try: + submission = Submission.objects.get(project=project, msgid=ref) + except Submission.DoesNotExist: + pass + + return submission def split_prefixes(prefix): """Turn a prefix string into a list of prefix tokens.""" @@ -487,6 +497,7 @@ def parse_mail(mail, list_id=None): date = find_date(mail) headers = find_headers(mail) pull_url = find_pull_request(message) + parent = find_submission_for_ppatch(project, None if not refs else refs[0]) # build objects @@ -510,6 +521,7 @@ def parse_mail(mail, list_id=None): diff=diff, pull_url=pull_url, delegate=delegate, + parent=parent, state=find_state(mail)) patch.save() LOGGER.debug('Patch saved') diff --git a/patchwork/models.py b/patchwork/models.py index 521b20c..bcc8f90 100644 --- a/patchwork/models.py +++ b/patchwork/models.py @@ -298,6 +298,7 @@ class Submission(EmailMixin, models.Model): # submission metadata name = models.CharField(max_length=255) + parent = models.ForeignKey('self', null=True, blank=True, related_name="children") # patchwork metadata diff --git a/patchwork/templates/patchwork/submission.html b/patchwork/templates/patchwork/submission.html index 088cceb..8f1386d 100644 --- a/patchwork/templates/patchwork/submission.html +++ b/patchwork/templates/patchwork/submission.html @@ -108,6 +108,20 @@ function toggle_headers(link_id, headers_id) </div> {% endif %} +{% if patchseries %} +<div class="patchform patchform-series"> +<h3>Patch Series</h3> + +<ul> +{% for patch in patchseries %} + <li><a href="{% url 'patch-detail' patch_id=patch.id %}">{{ patch.name }}</a></li> +{% endfor %} +</ul> + +</div> +{% endif %} + + {% if createbundleform %} <div class="patchform patchform-bundle"> <h3>Bundling</h3> diff --git a/patchwork/views/patch.py b/patchwork/views/patch.py index 3346568..cd73d8b 100644 --- a/patchwork/views/patch.py +++ b/patchwork/views/patch.py @@ -32,6 +32,35 @@ from patchwork.forms import PatchForm, CreateBundleForm from patchwork.models import Patch, Project, Bundle, Submission from patchwork.views import generic_list, patch_to_mbox +def find_patch_series(patch): + def find_child(parent): + series = [] + + subms = parent.children.all().only('id', 'name') + for subm in subms: + tmp = {} + + tmp['id'] = subm.id + tmp['name'] = subm.name + series.append(tmp) + + if subm.children.all(): + series += find_child(subm) + + return series + + # find the top parent + parent = patch + while parent.parent: + parent = parent.parent + + patches = find_child(parent) + if len(patches) == 0: + return None + + # We assume the fisrt submission in a series is a patch, and + # url will automaticly switch to cover if a patch is not found + return [{'id':parent.id, 'name':parent.name}] + patches def patch(request, patch_id): # redirect to cover letters where necessary @@ -104,6 +133,7 @@ def patch(request, patch_id): context['patchform'] = form context['createbundleform'] = createbundleform context['project'] = patch.project + context['patchseries'] = find_patch_series(patch) return render(request, 'patchwork/submission.html', context)
Added a ForeignKey in submission, and linked releated patches by 'In-Reply-To' or the first 'References'. Child emails must fill 'In-Reply-To' or 'References' with correct msg-id, or the parent field will keep empty. Signed-off-by: WEN Pingbo <wengpingbo@gmail.com> --- htdocs/css/style.css | 4 ++-- patchwork/bin/parsemail.py | 12 +++++++++++ patchwork/models.py | 1 + patchwork/templates/patchwork/submission.html | 14 +++++++++++++ patchwork/views/patch.py | 30 +++++++++++++++++++++++++++ 5 files changed, 59 insertions(+), 2 deletions(-)