search

brainhackorg / global2021

Website for Brainhack global 2021
https://brainhack.org/global2021/
MIT License
3 stars 11 forks source link

ENH: Refactor issues to pages script #83

Closed jhlegarreta closed 2 years ago

jhlegarreta commented 2 years ago

Refactor issues to pages script: split the existing script into a module to host the methods, and the actual script that makes use of such methods.

Paves the way towards an easier maintenance.

jhlegarreta commented 2 years ago

Related to #25.

Will rebase into main when #79 and #82 are merged, and will mark as Ready for review then.

Once this one gets merged, I'll add testing to avoid issues like #78, or at least to warn contributors about inconsistent things whenever something changes. Eventually, I think best will be to move the module to a separate repository so that it can be used across years without duplicating the code.

Remi-Gau commented 2 years ago

Eventually, I think best will be to move the module to a separate repository so that it can be used across years without duplicating the code.

I very much approve.

Was wondering if it would not be a nice thing to add type annotation in that python code. Can make your doc strings a wee bit shorter and usually helps make the code more readable.

jhlegarreta commented 2 years ago

Was wondering if it would not be a nice thing to add type annotation in that python code. Can make your doc strings a wee bit shorter and usually helps make the code more readable.

IMHO most urgent is to finish the draft PRs, get them merged and move the code into a separate repository. An issue can be opened if necessary to remind us about adding type hints, and be done in a separate PR.

FWIW, the documentation style used is NumPy-like due to a personal preference over Google-like, and NumPy itself has not yet adopted the necessary changes to support it within their documentation.

Remi-Gau commented 2 years ago

IMHO most urgent is to finish the draft PRs, get them merged and move the code into a separate repository. An issue can be opened if necessary to remind us about adding type hints, and be done in a separate PR.

Agreed. My suggestion was more in the "nice to have" section.

FWIW, the documentation style used is NumPy-like due to a personal preference over Google-like, and NumPy itself has not yet adopted the necessary changes to support it within their documentation.

No worries about that format of doc string. Was more thinking that you can have this format AND type annotation that can be used by static analysis tools like pylance. But as said above. No rush.