El mié., 21 nov. 2018 a las 12:11, Mads Kiilerich (<[email protected]>) escribió: > > On 11/20/2018 09:32 PM, Thomas De Schampheleire wrote: > > # HG changeset patch > > # User Thomas De Schampheleire <[email protected]> > > # Date 1541709517 -3600 > > # Thu Nov 08 21:38:37 2018 +0100 > > # Node ID 7350230aa3578c3fc3a4aff864e35a02eea216e1 > > # Parent 57b9fe6c7871e1ee74eca9c6723ccc4a9dfd5c77 > > controllers: forward pullrequests.delete_comment to changeset > > > > Remove duplication between pullrequests and changeset. > > We move the code outside ChangesetController to make it callable from > > PullrequestsController. > > > > Note: > > - instead of keeping the method pullrequests.delete_comment itself and > > letting it forward to changeset.delete_comment, an alternative solution > > would have been to change the routing table directly. However, the chosen > > solution makes it more explicit which operations are supported on each > > controller. > > > > diff --git a/kallithea/controllers/changeset.py > > b/kallithea/controllers/changeset.py > > --- a/kallithea/controllers/changeset.py > > +++ b/kallithea/controllers/changeset.py > > @@ -187,6 +187,22 @@ def create_comment(text, status, f_path, > > > > return comment > > > > +def delete_cs_pr_comment(repo_name, comment_id, pr_comment=False): > > + co = ChangesetComment.get_or_404(comment_id) > > (It would be nice to have more consistent "default" naming of variables > if different types, both in code and tests. "co" for ChangesetComment > doesn't seem obvious. "cc" seems like a better mnemonic. (But also, > ChangesetComment is perhaps badly named after it also is used for PRs > (which also are badly named)...))
The 'co' is for 'comment' and also used in templates and model: kallithea/templates/changeset/changeset_file_comment.html kallithea/model/comment.py so in that sense it is actually consistent naming. But I'm fine with other names, like 'comment' in full. > > > + if co.repo.repo_name != repo_name: > > + raise HTTPNotFound() > > + if pr_comment and co.pull_request.is_closed(): > > Do we need this pr_comment flag? Can't we just check co.pull_request? You're right, I'll remove it... > > > + # don't allow deleting comments on closed pull request > > + raise HTTPForbidden() > > /Mads _______________________________________________ kallithea-general mailing list [email protected] https://lists.sfconservancy.org/mailman/listinfo/kallithea-general
