[PATCH 0 of 9] remove UI notification feature

[Thomas De Schampheleire]

El jue., 6 dic. 2018 a las 12:26, Mads Kiilerich
(<mads at kiilerich.com>) escribió:
>
> Thanks.

Thanks for pushing so quickly.

>
> I wonder if some css also could be cleaned up?

Do you have specific examples or details? I searched for 'notif' but
found nothing.

>
> And we have so much cruft in upgraded databases anyway. I think we
> either should accept a bit more tech debt there and rely on a future
> "only preserve schema things we use now" database migration option, or
> just add a migration step now to drop these tables. I don't think the
> comment helps - it is not a list that can be trusted anyway.

Dropping tables now means that people that upgrade then downgrade
(changed their mind) will have missing data.
I thought it was common to leave things if they don't hurt.

I would be fine with a "only preserve schema things we use now"
database migration option if it happens roughly once, but it would
imply we also fix any other problems in the database schema.

>
> About the if condition indentation: I think we have the same deficiency
> in many other places. Perhaps, when the number of outstanding patches
> are at a low, we should just start using
> https://black.readthedocs.io/en/stable/the_black_code_style.html .

Have you used this in practice somewhere?
I did a quick run on our controllers, and while it has the benefit of
consistency, some of the long-line wrapping principles are not how I
would write it naturally. So in order to make it effective I'd need a
commit hook that fixes my code on the fly. In general, I would prefer
a style that we can reasonably adopt naturally without any hook
needing to do fixups.
But, I'm not making any vetos right now.

Best regards,
Thomas