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

remove python2, annotate #29

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Conversation

a-detiste
Copy link
Contributor

this is a draft, open to disscussion.

work can be split into two PR for an easier review:

  • remove Python2 support (first)
  • add annotations

Copy link
Owner

@fmenabe fmenabe left a comment

Choose a reason for hiding this comment

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

Hi, I review the PR. I have not much to say about type hints, I don't use them (yet) personally. I review hints and they seem coherents but I don't even know how to ensure they are correct (I used mypy on the file but not sure that's enough).

I also made comments about the addition of some variables instead of keeping the variables reassignments. Just curious is there a reason for this?

dokuwiki.py Show resolved Hide resolved
dokuwiki.py Show resolved Hide resolved
dokuwiki.py Show resolved Hide resolved
dokuwiki.py Show resolved Hide resolved
@a-detiste a-detiste reopened this Nov 2, 2023
@a-detiste
Copy link
Contributor Author

Please merge as-is ... I don't have much time anymore for annotations. But it is an incremental work anyway.

Greetings

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.

2 participants