Review: Approve

13:37 < nessita> cjwatson, question about your branch: the commit says "skip 
owner references..." but the code adds them, so I'm not sure I understand what 
is going on
13:39 < cjwatson> nessita: It adds them to a set called "skip" :)
13:41 <    cjwatson>| close-account's job is to do as full an erasure of an 
account as is possible (to satisfy legal demands etc.) while not breaking 
referential integrity.  There are way too many FKs on Person.id to be realistic 
to decide what to do about all of them up-front, so the purpose of the "skip" 
set is to be a list of FKs that we've determined are OK to keep around even 
after an account has been wiped ...
13:41 <    cjwatson>| ... and replaced with a minimal placeholder.
13:43 < nessita> cjwatson, I understand, thanks. And the tests, do they have a 
machinery where by adding an owner there are asserts that will ensure is not 
there?
13:45 < cjwatson> nessita: Indeed.  With only the test changes, one gets 
https://paste.ubuntu.com/p/DcKHgpfRPn/
13:45 <    cjwatson>| I recently refactored close-account to be super-paranoid 
about what happens if any unhandled FKs are left over
13:46 <    cjwatson>| So we are forced to make some kind of decision about them
13:46 < nessita> cjwatson, great, it makes sense and I think it looks good

-- 
https://code.launchpad.net/~cjwatson/launchpad/close-account-message-owner/+merge/360056
Your team Launchpad code reviewers is requested to review the proposed merge of 
lp:~cjwatson/launchpad/close-account-message-owner into lp:launchpad.

_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to     : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

Reply via email to