Re: Code review for grammalecte

2021-05-27 Thread Romain Porte
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,
>



Re: Code review for grammalecte

2021-05-25 Thread 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,

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



OpenPGP_signature
Description: OpenPGP digital signature