← Back to team overview

launchpad-reviewers team mailing list archive

Re: [Merge] lp:~jtv/launchpad/test-pofile-permissions into lp:launchpad/devel

 

> Right, looks good. That was quite painful to review :)
> 
> It was a little mind-bending, but I liked how you managed to keep a
> lid on complexity in test_translationpermission.
> 
> One miniscule comment/question.
> 
> 
> [1]
> 
> +    def test_admin_can_edit(self):
> +        # Administrators can edit all translations and make suggestions
> +        # anywhere.
> +        self.closeTranslations()
> 
> Why is closeTranslations() needed? Can you explain either here or in
> its docstring (assuming my question isn't stupid).

Good question, actually.  It is needed to show that an admin has these rights _despite the chosen access model tending towards keeping people out_.  Translations are open by default, and under those circumstances testing that an admin has edit rights has little value.
-- 
https://code.launchpad.net/~jtv/launchpad/test-pofile-permissions/+merge/40058
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/test-pofile-permissions into lp:launchpad/devel.



References