Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🧫 Ajout d'un linter Python #35

Merged
merged 3 commits into from
Aug 27, 2022
Merged

🧫 Ajout d'un linter Python #35

merged 3 commits into from
Aug 27, 2022

Conversation

Aeris1One
Copy link
Collaborator

@Aeris1One Aeris1One commented Aug 22, 2022

Cette PR est soumise à discussion quant-aux réglages.
Dans l'état actuel, le linter vérifie non seulement que le code Python est correct (pas de fonction appelée non définie par exemple), indique les morceaux de code inutiles (une fonction importée mais non utilisée) mais ausi vas reformater selon la PEP-8 pour que le code soit lisible par les personnes habituées à travailler en PEP-8.

Je pense que ça permettrai d'avoir un code bien plus uniforme, là où je sais que chacun à ses petites habitudes. Cependant il y avait déjà eu le débat sur Discord à propos de certains formatages, pas forcément autorisés par la PEP-8 qui semble pour certains plus clairs que d'autres.

Que faisons-nous ?

@ascpial
Copy link
Contributor

ascpial commented Aug 22, 2022

Je penses que il ne faudrait pas mettre de formatage automatique pour la pep-8, vu que les collaborateurs principaux ne souhaitent pas l'utiliser. De toute manière, si il faut l'ajouter on pourra toujours le faire plus tard.
Par contre la vérification de code peut être une bonne idée, même si une PR avec du code contenant des erreurs me semble assez stupide...

@VForiel
Copy link
Contributor

VForiel commented Aug 22, 2022

Le passage en pep8 peut être intéressant pour les releases, mais en effet je suis d'accord avec ascpial, pendant les phases de dev, chacun bosse sur son système avec ses propres habitudes

@Aeris1One
Copy link
Collaborator Author

Justement, pendant ton développement tu fais ce que tu veux, mais dès que tu part sur la branche béta ou la branche master il formate en suivant la pep8
Et puis ça arrive de push du code avec des erreurs, c'est même très fréquent, vaut mieux un garde-fou ascpial :)

@VForiel
Copy link
Contributor

VForiel commented Aug 23, 2022

Dans ce cas c'est bon pour moi

@Aeris1One Aeris1One marked this pull request as draft August 25, 2022 17:23
@Aeris1One
Copy link
Collaborator Author

L'objectif du linter est d'obtenir un code bien plus lisible par les humains, par exemple, les premières lignes du start.py donneraient, avec seulement le linter black:
image

@Aeris1One
Copy link
Collaborator Author

Je suis en train de faire quelques tests d'efficacité de mon côté, j'ai donc repassé la PR en WIP

@ascpial
Copy link
Contributor

ascpial commented Aug 25, 2022

Si ça peut aider les autres à utiliser la pep 8 why not du coup xD

@Aeris1One
Copy link
Collaborator Author

Très sérieusement, si des trucs sont contre la PEP8 mais nous conviennent mieux que ce que dit la PEP8 et/ou sont plus lisible, on les gardera

@Aeris1One
Copy link
Collaborator Author

Donc, j'ai laissé comme linters :

  • Black, qui utilise sa propre feuille de style, inspirée de la PEP8 mais aussi d'autres conventions comme le Google Python Style Guide, pour rendre le code bien plus lisible (voir la screen ci-dessus)
  • PyLint, qui détecte les erreurs dans le codes, mais également quelques erreurs de style

Black fait de l'auto-fix, il fera un commit dans chaque PR et à chaque push dans master et beta pour s'assurer que tout est ok !
PyLint ne fait pas d'autofix, et je suis en train de me battre avec lui (la note du bot est d'environ 5/10 actuellement)

@Aeris1One Aeris1One marked this pull request as ready for review August 26, 2022 10:14
@Aeris1One Aeris1One requested a review from VForiel August 26, 2022 10:14
@VForiel
Copy link
Contributor

VForiel commented Aug 26, 2022

LGTM mais idk comment tester ça 😅

@Aeris1One
Copy link
Collaborator Author

Aeris1One commented Aug 26, 2022

Je l'ai déjà testé de mon côté à plusieurs reprises, il tourne sur la branche master de Aeris1One/Gipsy : https://github.com/Aeris1One/Gipsy/actions/runs/2928876343

Black est en failed à cause des "import bot import checks" qui sont foirés. J'ai ouvert un PR, voila ce que ça donne sur une PR : https://github.com/Aeris1One/Gipsy/pull/1

@ascpial
Copy link
Contributor

ascpial commented Aug 26, 2022

Aller aller on révise faut avoir des meilleurs notes

@Aeris1One
Copy link
Collaborator Author

Je vais ouvrir une autre PR pour le refactor, j'ai réussi à faire passer start.py de 6.75 à 10/10 pour l'instant, et utils est en bonne voie.
Le gros problème est que certaines fonctions redéfinissent des accès globaux (beaucoup de fonctions prennent un argument noté "config" par exemple, qui est défini globalement, et le linter aime pas du tout qu'on l'overwrite localement)

@VForiel
Copy link
Contributor

VForiel commented Aug 26, 2022

Ca m'a l'air tout bon, mais y'a tellement de soucis dans le code que PyLint et Black galèrent d'après les logs de l'action 😂

@Aeris1One
Copy link
Collaborator Author

Aeris1One commented Aug 26, 2022

Le deuxième exemple (dans la PR) crash parce qu'évidemment mon fork peut pas push dans l'upstream (la PR étant Gunivers -> Aeris1One) mais ici normalement il n'y aura pas de problème.

Black crashait à cause des "import bot import checks" donc il n'arrivais pas à revoir le style de ces fichiers totalement incorrects, il aurait dû exit sans erreur. PyLint exit avec une erreur et c'est totalement normal puisque le score est inférieur à 8/10.

@Aeris1One
Copy link
Collaborator Author

Si cette PR est merge, le résultat de l'action devrait ressembler à ça : https://github.com/Aeris1One/Gipsy/runs/8034509506

@Aeris1One
Copy link
Collaborator Author

Le check des conventions et des messages d'information de style sont désactivés.

Une seconde Action en mode dégrade à été créee de manière à pouvoir fonctionner sur des PRs provenant de forks (et sur lesquelles on a pas de droit en écriture, donc pas possible d'autofix). Pour ces PR, l'autofix se fera au moment du merge.

@VForiel VForiel merged commit 47ff04d into Curiosity-org:master Aug 27, 2022
Aeris1One pushed a commit that referenced this pull request Apr 25, 2023
* fix(stats): NaN bot latency

* fix: segmentation fault

* fix(stats): wait until bot is ready

* fix(tickets): topics without emojis

* fix(tickets): opening a ticket

* refract(welcomer): welcome messages

* feat: enable translations cog on boot

* refract(emoji): emojis cog and manager

* fix(tr): add missing translations

* refract(args): unicode emoji converter

* feat(tickets): allow unicode emojis in topics

* fix(tr): weird english translations

* refract(events): sending server counts to lists

* add more variables to voice channels

* feat(antiscam): add report system

* feat(antiscam): add doc link to report cmd

* feat(tickets): disable tickets names "other"

* refract(rss): use class instead of dict

* fix(rss): multiple small issues

* feat(Zbot): typehint main cogs in `get_cog()`

* fix(linter): typehints

* fix(stats): sending stats to deleted bots list

* feat(mc): makes `mc mod` compat. with CurseForge

* doc: v4.1.1

* feat(rss): guild inactivity protection

* feat(linter): new allowed var name

* refract(mc): server data collection code linting

* feat(roles_react): use proper order when updating

* fix(tr): missing cap in French translation

* feat(tr): filter deleted translations when merging

* fix(antiscam): interaction detection

* feat(serverlogs): add tickets creation logs

* feat(admin): new database cmd

* build(lib): upgrade d.py version

* feat(error): add app interactions error handler

* fix(tickets): too wide interaction detection

* fix(conversion): discord IDs length

* fix(softban): reason containing spaces

* fix(doc): rss add documentation

* fix(rss): stopping rss loop

* feat(xp): new levelup message

* fix(tr): tr status with language

* refract(args): literal arguments

* feat(perms): allow binary/integer perms

* refract(server): remove unusued config

* feat(usernames): disable records for large guilds

* feat(info): change usernames history tip

* feat(ticlets): custom format

* feat(reminders): delete nice select menu

* refract(stats): improve latency frequency

* refract(stats): remove useless try/except

* refract(reminders): cleanup database methods

* fix(db): logging SQL errors

* refract(stats): use internal bot servers count

* feat(info): show bot owners when available

* feat(tr): update guild features translations

* fix(doc): misleading doc for ragequit

* feat(fun): yet another useless command

* dep(emoji): update emoji lib version

* feat(stats): count antiscam actions

* fix(dep): bot startup

* feat(logs): add new modlogs

* feat(logsà: add member_verification

* doc(moderation): deprecate verification system

* feat(mc): use curseforge api for mods

* feat(error): handle event errors

* feat(modlogs): add log when welcomer fails

* feat(modlogs): add logs when rss fails

* feat(tickets): pin intro message

* feat(stats): add ticket creation stats

* feat(stats): add nickname records

* feat(stats): add translations

* refract(config): rename some old options

* fix(tr): wrong help translation

* feat(boot): add cogs loading progress bar

* fix(ping): security patch

* build(lib): update LRFutils URL

* Update Crowdin configuration file

* Update Crowdin configuration file

* Update Crowdin configuration file

* doc(verification): add depreciation warning

* fix(tr): translation file merging

* feat(tr): update translations

* fix(tr): file un-flattening with lists

* fix(tickets): topics list with no default

* fix: progress bar

* feat(tr): add new translation language

* feat(stats): add rss warnings

* fix(args): simplify oauth url regex

* fix(ping): regex detection

* fix(arg): discord ID argument

* refract(lint): fix some code QL issues

* First hindi translation (#20)

* Create hi.json

* Remove issues badge from readme

* refract(lint): broad except

* fix(args): bot invite regex

* feat(reload): log when a lib is reloaded

* refract(lint): remove broad excepts

* refract(tr): cleanup languages files

* doc(readme): add crowdin badge

* New Crowdin updates (#24)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (French familiar)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (French familiar)

* New translations en.json (French familiar)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (French familiar)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (French familiar)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (French familiar)

* New translations en.json (LOLCAT)

* New translations en.json (French familiar)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (French familiar)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (French familiar)

* New translations en.json (LOLCAT)

* New translations en.json (French familiar)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* remove crowdin weird lolcat files

* fix(tr): revert fr translations

* fix(config): nicknames history

* fix(config): nicknames history

* fix(logs): reset option

* fix(tr): wrong translations

* fix(reminder): correct error handler

* feat(logs): add join date to verified log

* feat(logs): fixed date delta

* New Crowdin updates (#26)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (French familiar)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (French familiar)

* New translations en.json (French familiar)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (French familiar)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (French familiar)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (French familiar)

* New translations en.json (LOLCAT)

* New translations en.json (French familiar)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (French familiar)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (French familiar)

* New translations en.json (LOLCAT)

* New translations en.json (French familiar)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* remove crowdin weird lolcat files

* fix(tr): revert fr translations

* fix(config): nicknames history

* refract(lint): errors and list creation

* feat(find): add servers count

* feat(deps): update discord.py version

* fix(rss): youtube names with spaces

* fix(rss): youtube custom names wrongly detected

* fix(args): unicode emojis

* refract(tickets): delay cooldown check a bit

* refract(tr): delete unusued code

* feat(slash): first slash command

* fix(errors): error handler with interactions

* fix(help): too many subcommands

* fix: errors related to interactions

* fix(errors): some errors being sent 2 times

* refract: remove unusued import

* feat(slash): start migrating to app commands

* refract(moderation): cleanup max roles affectation

* doc(embed): try to fix characters escape

* fix(language): use them instead of him

* refract(db): remove one database connection

* refract(lint): rename imports

* refract(lint): remove broad excepts

* feat(slash): use slash commands for /find

* refract(errors): better error for slash in beta

* fix(utils): remove dead bots list API

* New Crowdin updates (#27)

* Update source file en.json

* Update source file en.json

* Update l10n code (#28)

* feat(find): add servers count

* feat(deps): update discord.py version

* fix(rss): youtube names with spaces

* fix(rss): youtube custom names wrongly detected

* fix(args): unicode emojis

* refract(tickets): delay cooldown check a bit

* refract(tr): delete unusued code

* feat(slash): first slash command

* fix(errors): error handler with interactions

* fix(help): too many subcommands

* fix: errors related to interactions

* fix(errors): some errors being sent 2 times

* refract: remove unusued import

* feat(slash): start migrating to app commands

* refract(moderation): cleanup max roles affectation

* doc(embed): try to fix characters escape

* fix(language): use them instead of him

* refract(db): remove one database connection

* refract(lint): rename imports

* refract(lint): remove broad excepts

* feat(slash): use slash commands for /find

* refract(errors): better error for slash in beta

* fix(utils): remove dead bots list API

* fix(error): conversion error catching

* fix(rss): feeds selector callback

* fix(stats): infinite api latency

* feat(rss): allow batch edition for text + mentions

* fix(tr): translations in threads

* fix(roles_react): external emojis usage

* feat(stats): add stats cmds command

* feat(help): better aliases indication

* New Crowdin updates (#29)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* Update source file en.json

* Update source file en.json

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* feat(stats): improve `stats cmds`

* doc: up version to 4.1.2

* feat(stats): hide remindme stats

* refract: misc. improvements

* fix(admin): update log mentions

* fix(roles_react): update with no specified emojis

* fix(interactions): wrong method name

* fix(tickets): unicode emojis again

* feat(threads): allow threads instead of channels

* feat(modlogs): separate beta from release

* fix(stats): antiscam internal stats

* fix(admin): reactions to admin commands

* feat(tickets): add topics limit + batch deletion

* feat(args): add months as duration + fix years

* fix(formats): timem delta formatting

* feat(reminders): improve selection timeout

* feat(stats): add total guilds count

* feat(stats): add sent xp cards to internal stats

* fix(stats): rss internal stats

* Hindi Translations (#31)

* Create hi.json

* Update source file en.json (#32)

* fix(tr): hi translations

* New Crowdin updates (#33)

* New translations en.json (Hindi)

* New translations en.json (Hindi)

* New translations en.json (Hindi)

* New translations en.json (Hindi)

* New translations en.json (Hindi)

* fix(interaction): wrong attribute

* fix(sql): SELECT query detection

* fix(stats): commands usage SQL query

* fix(usernames): dates field in embed

* fix(pep8): markdown usage

* fix(antiscam): weird discord behavior

* feat(logs): add years in verification/leave logs

* remove(moderation): custom verification system

* refract: split libs.classes file

* refract(pep8): method names

* feat(reminders): new selector when batch deleting

* feat(rss): use new paginated selector

* feat(info): add ID length to `info id`

* fix(help): JSON file format

* refract(hw): halloween-related code

* feat(admin): add logs when syncing commands

* refract(logs): allow passing 1 embed only

* feat(event): add halloween event

* feat(hw): convert hw commands to hybrid

* refract(errors): linting and things

* feat(error): better error response

* refract(hw): remove useless import

* feat(rss): add feed enable/disable commands

* feat(stats): add disabled rss, remove translations

* refract(rss): prettify some code

* feat(stats): add dailies points

* fix(stats): is_sum column

* feat(antiscam): log antiscam warnings

* feat(paginator): add pages count

* doc(perms): update Discord link about admin perm

* feat(events): halloween 2022

* refract(event): standardize random reaction system

* fix(admin): commands response time

* refract(events): better-looking code

* refract(events): event rank card system

* feat(admin): add commands for rank cards

* refract(event): points and colors

* feat(event): actually correct halloween 2022 date

* refract: remove translations cog loading

* fix(paginator): unselecting items

* feat(rss): disable feeds with too many errors

* refract(rss): remove duplic. disabled feeds stats

* feat(stats): add event points stats

* feat(event): add leaderboard to rank command

* feat(fun): add movie command

* doc: v4.1.3

* New Crowdin updates (#34)

* New translations en.json (Hindi)

* New translations en.json (LOLCAT)

* Update source file en.json

* New translations en.json (LOLCAT)

* Update source file en.json

* Update source file en.json

* Update source file en.json

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* Update source file en.json

* Update source file en.json

* Update source file en.json

* Update source file en.json

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

* New translations en.json (LOLCAT)

Co-authored-by: Devendra Poonia <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants