launchpad-reviewers team mailing list archive

[Natalia Bidart <natalia.bidart@xxxxxxxxxxxxx>]

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.