[PATCH 0 of 9] remove UI notification feature

[Mads Kiilerich]

On 12/6/18 1:02 PM, Thomas De Schampheleire wrote:
>> 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.


We will have to establish how we want to use database migrations. I 
guess it only will be possible to make the most simple migrations fully 
reversible. And even in that case, it is not necessarily clear if 
re-applying a migration step should do that migration "from scratch" 
(like create a new table, empty and fully correct) or pick it up where 
it was before (with old data).

I guess I just would expect the bi-directional migration steps to give a 
correct schema and consistent data. I would not expect the migration to 
"take a backup" just to allow a down-migration could "restore it".

Anyway: Ok to not drop the old unused tables and relational references. 
But I wonder how much value it has to maintain a catalogue of "backups" 
to delete later. And especially if it should be as comments spread out 
in db.py .


> 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.


Yes, there is other tech debt. Unused stuff that not necessarily have 
been dropped. For example, I think there might be a need for ensuring 
the right field types, indices and constraints.


>> 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.


I don't like it's formatting very much, so it's not about advocating my 
preference. I mainly like that it leaves very little to discuss.

Yes, it would perhaps require some kind of hook. But also, formatting 
can always be applied later, like when landing the changes. We already 
have `scripts/run-all-cleanup` which I sometimes remember to run ;-)

/Mads