[PATCH 1/2] dim: Workaround Outlook + Patchwork mess.

[Arkadiusz Hiler]

On Fri, Feb 16, 2018 at 10:59:50AM -0800, Rodrigo Vivi wrote:
> On Fri, Feb 16, 2018 at 10:52:11AM +0200, Arkadiusz Hiler wrote:
> > On Thu, Feb 15, 2018 at 11:14:09AM -0800, Rodrigo Vivi wrote:
> > > Patchwork has a own identity that doesn't respect the author configuration.
> > > It automatically changes the author to whatever it was last seen on
> > > the public mailing list.
> > > 
> > > In case the response came from an outlook server the author identity
> > > inside patchwork will get changed to Last, First.
> > > 
> > > We have authors that respond emails to public mailing list using
> > > Intel emails. Even non outlook users can end up having this trouble
> > > because the Evolution can be setup with EWS, for instance.
> > > 
> > > Since we could never fix patchwork for real to respect users
> > > choice we can workaround it here when applying the patch.
> > 
> > This statement is not true. It's not quite trivial, but definitely possible.
> 
> I'm sorry for the way I wrote it. I didn't mean to do any critic.
> I'd be glad in re-phrase that to:
> 
> Since the solution on patchwork to respect user's name of choice
> is not trivial, we can, for now, workaround this here when applying the patch.
> 
> > 
> > This models needs additional field for email address:
> > https://github.com/dlespiau/patchwork/blob/master/patchwork/models.py#L308	
> > 
> > This part of code needs to save the raw author information using the
> > newly added field:
> > https://github.com/dlespiau/patchwork/blob/master/patchwork/bin/parsemail.py#L759
> > (now it's just updating the name in the association)
> 
> It's the first time that I ever look to this code so please bare with me.
> But is it possible here to differentiate from mail mail and patch mail?
> On this case we could only save the name from the patch mail and never from mail mail.
> 
> This would solve the issue since smtp itself used by git send-email doesn't change
> any address. Only the EWS does.

That's one way to solve this, but it would still be just a half of a
solution.

There are still edge cases that can bite us in the future - legal change
of the name, someone who has a typo in the gitconfig just on one of the
machines - that ends up propagating to other patches (I've seen this
happening!).

That's why I am strongly in favor of keeping "From" with each of the
patches - it matches intuitive understanding of patch being self
contained.

> > Then you need either create a fall-back mechanism in the model so
> > patches that came in before the change will still use data from the
> > foreign key to the Person model or create a migration that copies
> > current authorship information onto the patches.
> > 
> > And update how the mboxes are generated to use the name form the Patch:
> > https://github.com/dlespiau/patchwork/blob/master/patchwork/views/__init__.py#L218
> > 
> > There's no need of removing the "Person" concept, as it also serves
> > other purposes (search and ownership management).
> 
> I agree that that is useful.
> 
> > 
> > Also write a few tests to make sure it works as expected.
> 
> Do you have any example of tests case?

Tests are in patchwork/tests/ subdirectories.

You would quite likely want to implement new tests for "mboxviews" to
make sure which person is used in the From line in the generated mbox.

And another one for mail parser to make sure it parses and handles
authorship correctly.

Those are just regular django unit tests. You can look up details in
their documentation.

Please base you work of this branch:
https://github.com/ivyl/patchwork/commits/django111

> And any doc to show how to setup a local version to test it?

Check out the docs/ directory. There is installation and development
guide.

> Is there a test version and production version?

I am running one internal instance locally that I use to test the
changes with real workloads from mailing lists. I would deploy it there
for a couple of days before going with freedesktop deployment.

> > Patches are welcome. Repository has CI set up. I have commit right to
> > this project. I can review and accept pull requests. I can also deploy
> > the update.
> 
> Is Damien still maintaining this? Or should we move to somewhere else?

Sadly I am the only contributor as of now, and I would love to have
someone else to review my work especially if that person helps me with
the development from time to time.

I have my own fork on github, but I always push the changes to Damien's
repo when they are "production ready".

> Is there any plan to contribute back to "main" getpatchwork? Or it will
> be a forever fork?

Quite likely this is not going to happen in the foreseeable future.
Freedesktop forked quite a while ago. I was told that back then the
patchwork project was not very keen on the direction of changes - UI
revamp, automatic grouping patches in series and REST API instead of
just XMLRPC, hence the fork.

Now they are starting to catch up with features, but IMO they are in
much rougher shape, usability-wise. Series are a second-class concept
and are not nearly as functional as what freedesktop has, and the
REST API looks very different and misses some of the key pieces.

IMHO it's less effort to maintain the fork.

- Arek