[PATCH 08/15] series: Create Series objects when parsing mails

Finucane, Stephen stephen.finucane at intel.com
Tue Oct 20 11:33:19 AEDT 2015


> On Sat, Oct 10, 2015 at 12:21:17AM +0100, Finucane, Stephen wrote:
> > > +# The complexity here is because patches can be received out of order:
> > > +# If we receive a patch, part of series, before the root message, we
> > > create a
> > > +# placeholder series that will be updated once we receive the root
> > > message.
> > > +def find_series_for_mail(project, name, msgid, refs):
> > > +    if refs == []:
> > > +        root_msgid = msgid
> > > +    else:
> > > +        root_msgid = refs[-1]
> > > +
> > > +    try:
> > > +        revision = SeriesRevision.objects.get(root_msgid = root_msgid)
> > > +        series = revision.series
> > > +        if name:
> > > +            series.name = name
> > > +    except SeriesRevision.DoesNotExist:
> > > +        if not name:
> > > +            name = "Untitled series"
> >
> > Nope. This is a lie as the name of the series is not 'Untitled
> > series': it's None (i.e. nothing). The model needs to be updated to
> > allow 'null' names and this 'Untitled series' string should be
> > generated in the UI.
> 
> I went back and forth on this. Making the string part of the client code
> instead of having a stronger invariant "all series have a name" make
> things harder: special handling in all clients (web site, but also
> command line clients), special handling when editing (I have further
> work allowing people to edit their series name through the web UI), ...
> 
> The real thing I could find to leave the string outside of the DB is for
> i18n, but I have no intention to go there.

Hmmm, I understand: it's a tough one to decide on. Just to confirm, we don't use names to uniquely identify series, correct? If so, it would probably be valid to return an return an empty string to the user when using the API? There would be a little modification needed to the web UI but this should be only take a few lines. Referring back to the biggest collection of Python code I know, both OpenStack Nova/Neutron allow for empty instance/network names respectively given that they already have a UUID to identify these elements.

However, all this matters little without something like i18n as you point out. It would be nice-to-have from a DB perspective and a potential future i18n feature but it's not critical. If you'd like to rework this then please do. Otherwise:

Acked-by: Stephen Finucane <stephen.finucane at intel.com>


More information about the Patchwork mailing list