Thanks for your review! On Sat, Nov 17, 2012 at 12:52 AM, Jakub Wilk <jw...@debian.org> wrote:
> License in debian/copyright is not the same as in LICENSE. Fixed upstream (I believe) — is that what you meant: http://bazaar.launchpad.net/~mitya57/python-markups/trunk/revision/87/LICENSE ? > Shouldn't python3-pygments be in Recommends? python3-markdown and python3-docutils already recommend it, I don't see why should it be recommended explicitly > I'd use "filter" rather than "findstring" in debian/rules. Fixed. > Apart from these, the packaging looks good. I have some comments to the > upstream part though: > >> CONFIGURATION_DIR = os.path.expanduser('~/.config/') > > Could you make it support XDG Base Directory Specification[0] instead of > hardcoding ~/.config? Thanks. > >> try: >> import markdown >> except: >> return False > > > Make it s/except:/except ImportError:/. [1] (There are more instances of the > same buglet in other files.) > >> orig_stylesheet = self.publish_parts(text)['stylesheet'] >> # Cut off <style> and </style> tags >> return orig_stylesheet[25:-10] + get_pygments_stylesheet('.code') > > Eww, magic numbers. Wouldn't it break horribly if the user disabled > embed_stylesheet in their docutils.conf? Fixed in upstream branch. >> start_position = head.find('<script ') >> end_position = head.rfind('</script>') >> if start_position >= 0 and end_position >= 0: >> mjurl = head[start_position:end_position+9]+'\n' >> return mjurl.replace(MATHJAX_WEB_URL, get_mathjax_url(webenv)) >> return '' > > Eww again. How about using regular expressions instead? :) 9 here is just len('</script>') — this will never change :) I'll make a new upstream release soon and then update the packaging. -- Dmitry Shachnev -- To UNSUBSCRIBE, email to debian-mentors-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/cakimphwitivnkbx6lqjtr0-ebytszhg0r7qgoyd1w7eyc9v...@mail.gmail.com
