1. Forum
  2. Usenet
  3. LINUX.DEBIAN.MAINT.PYTHON

  • Code review for grammalecte

    From =?UTF-8?Q?Louis-Philippe_V=c3=a9ron@21:1/5 to All on Tue May 25 23:00:02 2021
    Copy: debian-python@lists.debian.org

    This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --vUsIuwWARPhrZfEu91dyiB7zGzpgr5Asz
    Content-Type: text/plain; charset=utf-8
    Content-Language: en-US
    Content-Transfer-Encoding: quoted-printable

    Hello!

    This is quite a large package, so instead of continuing to spam the IRC channel, here are my comments.

    1. These files are licensed under the MPL2

    * darg.py
    * gc_core/py/oxt/Grammalecte.py
    * gc_core/py/oxt/Options.py
    * gc_lang/fr/build_data.py
    * gc_lang/fr/dictionnaire/genfrdic.py
    * gc_lang/fr/dictionnaire/_templates/ooo/DictionarySwitcher.py
    * gc_lang/fr/oxt/AppLauncher.py
    * gc_lang/fr/oxt/Graphspell.py
    * gc_lang/fr/oxt/About/About.py
    * gc_lang/fr/oxt/ChangeAuthor/Author.py
    * gc_lang/fr/oxt/ContextMenu/ContextMenu.py
    * gc_lang/fr/oxt/DictOptions/DictOptions.py
    * gc_lang/fr/oxt/DictOptions/LexiconEditor.py
    * gc_lang/fr/oxt/DictOptions/SearchWords.py
    * gc_lang/fr/oxt/DictOptions/TagsInfo.py
    * gc_lang/fr/oxt/GraphicOptions/GraphicOptions.py
    * gc_lang/fr/oxt/Lexicographer/Enumerator.py
    * gc_lang/fr/oxt/TextFormatter/TextFormatterEditor.py
    * gc_lang/fr/oxt/TextFormatter/TextFormatter.py
    * graphspell/dawg.py
    * graphspell/progressbar.py
    * graphspell-js/dawg.js

    To me, this is a sign you haven't read the code

    Although it can be long and tiresome (and trust me, I know, I've just
    read the entire codebase :P), it's an important step in packaging
    something in Debian. When updating a package, you should also go through
    the diff.

    2. There should be an entry in d/copyright for gc_lang/fr/dictionnaire/metaphone2.py

    3. gc_lang/fr/nodejs/cli/lib/minimist.js seems to be a vendored copy of
    a version of node-minimist. If you need it, you should use the debian
    package. If not, it should be excluded.

    That's pretty much it! Fix those issues and I'll be happy to upload to experimental.

    Cheers,

    --
    ⢀⣴⠾⠻⢶⣦⠀
    ⣾⠁⢠⠒⠀⣿⡁ Louis-Philippe Véronneau
    ⢿⡄⠘⠷⠚⠋ pollo@debian.org / veronneau.org
    ⠈⠳⣄


    --vUsIuwWARPhrZfEu91dyiB7zGzpgr5Asz--

    -----BEGIN PGP SIGNATURE-----

    iHUEARYKAB0WIQTKp0AHB6gWsCAvw830JXpQshz6hQUCYK1koQAKCRD0JXpQshz6 haqzAP0cczojdEoCq7QiJFVxv2QLbvXG0XMt+ZEYvVjVFqEkVQEAjXrKiA3dBjdq 2JMNLq8bn1JBrWQCt2RTM3V1YyXY3gU=
    =U7wV
    -----END PGP SIGNATURE-----

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)
  • From Romain Porte@21:1/5 to All on Thu May 27 14:30:01 2021
    Hi pollo,

    Thanks for the review, I have (force)pushed the new changes with commits
    that address the issues listed here, as well as other issues from IRC
    comments you did earlier.

    Please let me know by reply or on IRC if something is still missing.

    Best regards,

    Romain.

    25/05/2021 22:57, Louis-Philippe Véronneau :
    Hello!

    This is quite a large package, so instead of continuing to spam the IRC channel, here are my comments.

    1. These files are licensed under the MPL2

    * darg.py
    * gc_core/py/oxt/Grammalecte.py
    * gc_core/py/oxt/Options.py
    * gc_lang/fr/build_data.py
    * gc_lang/fr/dictionnaire/genfrdic.py
    * gc_lang/fr/dictionnaire/_templates/ooo/DictionarySwitcher.py
    * gc_lang/fr/oxt/AppLauncher.py
    * gc_lang/fr/oxt/Graphspell.py
    * gc_lang/fr/oxt/About/About.py
    * gc_lang/fr/oxt/ChangeAuthor/Author.py
    * gc_lang/fr/oxt/ContextMenu/ContextMenu.py
    * gc_lang/fr/oxt/DictOptions/DictOptions.py
    * gc_lang/fr/oxt/DictOptions/LexiconEditor.py
    * gc_lang/fr/oxt/DictOptions/SearchWords.py
    * gc_lang/fr/oxt/DictOptions/TagsInfo.py
    * gc_lang/fr/oxt/GraphicOptions/GraphicOptions.py
    * gc_lang/fr/oxt/Lexicographer/Enumerator.py
    * gc_lang/fr/oxt/TextFormatter/TextFormatterEditor.py
    * gc_lang/fr/oxt/TextFormatter/TextFormatter.py
    * graphspell/dawg.py
    * graphspell/progressbar.py
    * graphspell-js/dawg.js

    To me, this is a sign you haven't read the code

    Although it can be long and tiresome (and trust me, I know, I've just
    read the entire codebase :P), it's an important step in packaging
    something in Debian. When updating a package, you should also go through
    the diff.

    2. There should be an entry in d/copyright for gc_lang/fr/dictionnaire/metaphone2.py

    3. gc_lang/fr/nodejs/cli/lib/minimist.js seems to be a vendored copy of
    a version of node-minimist. If you need it, you should use the debian package. If not, it should be excluded.

    That's pretty much it! Fix those issues and I'll be happy to upload to experimental.

    Cheers,


    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)