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

Correction des warnings et optimisation #38

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

kstzl
Copy link
Contributor

@kstzl kstzl commented Dec 19, 2023

  • Correction de certains warning
  • Suppression des includes inutiles
  • Bricolage dans LovyanGFX pour arrêter les warnings sur le alloca

Bref, plus de warnings lors de la compilation et c'est plus rapide. 😃

Copy link

@Zerbaib Zerbaib left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

j'ai pas tester la compilation car je n'ai pas ça chez moi mais le code me semble good

Copy link
Member

@DarkBrines DarkBrines left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pourquoi j'ai été ping pour review ? Je suis même pas sur ce projet 🫡

@Charlito33
Copy link
Member

Pourquoi j'ai été ping pour review ? Je suis même pas sur ce projet 🫡

My bad, vu que t'avais le rôle C++ sur le Discord, et que GitHub te proposait...

@hanako-eo
Copy link

Je viens de regarder les modifications avec un oeil curieux et pourquoi retirer autant de void dans les déclarations de fonction ?

Copy link
Member

@Charlito33 Charlito33 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay pour moi, compile sous Windows sans erreurs, ni warnings (gg) !

PS :
J'ai relu tous les changements,
Et on retrouve beaucoup "d'erreurs" qui sont en réalité des "Clang-tidy".
Ca pourrait être évité à mon avis ?

@kstzl
Copy link
Contributor Author

kstzl commented Dec 19, 2023

Je viens de regarder les modifications avec un oeil curieux et pourquoi retirer autant de void dans les déclarations de fonction ?

Car inutile

@kstzl
Copy link
Contributor Author

kstzl commented Dec 19, 2023

Okay pour moi, compile sous Windows sans erreurs, ni warnings (gg) !

PS : J'ai relu tous les changements, Et on retrouve beaucoup "d'erreurs" qui sont en réalité des "Clang-tidy". Ca pourrait être évité à mon avis ?

Oui il y à beaucoup de warnings, j'ai tenté d'en enlever le plus possible sans casser le reste 😀

src/smart_alloca.hpp Outdated Show resolved Hide resolved
@hanako-eo
Copy link

hanako-eo commented Dec 20, 2023

Je viens de regarder les modifications avec un oeil curieux et pourquoi retirer autant de void dans les déclarations de fonction ?

Car inutile

Alors pour le coup les void ont un sens, ce n'est pas "inutile" et ce n'est pas approprier au sujet de la PR, pour le coup, ils n'ont rien à faire dans cette PR ci. Après ça c'est pour gérer correctement le contenu des PR et ne pas avoir n'importe quoi, mais je ne fais pas de review donc voila 😅 ce n'est pas à moi de choisir ça.

@kstzl
Copy link
Contributor Author

kstzl commented Dec 20, 2023

Je viens de regarder les modifications avec un oeil curieux et pourquoi retirer autant de void dans les déclarations de fonction ?

Car inutile

Alors pour le coup les void ont un sens, ce n'est pas "inutile" et ce n'est pas approprier au sujet de la PR, pour le coup, ils n'ont rien à faire dans cette PR ci. Après ça c'est pour gérer correctement le contenu des PR et ne pas avoir n'importe quoi, mais je ne fais pas de review donc voila 😅 ce n'est pas à moi de choisir ça.

En C oui ça a une signification de ne pas avoir de void dans une fonction, ça veut dire que ça prend tout et n'importe quoi en argument. Or, là, on est en C++

C'est obsolète en plus.

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.

6 participants