-
Notifications
You must be signed in to change notification settings - Fork 25
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
Doc: flake8 todos #1981
Doc: flake8 todos #1981
Conversation
Hi @moe-ad! Could we separate this PR in two and have the license headers updated in a different PR? Otherwise it makes it difficult to review... |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1981 +/- ##
==========================================
- Coverage 88.48% 88.39% -0.10%
==========================================
Files 89 89
Lines 10251 10251
==========================================
- Hits 9071 9061 -10
- Misses 1180 1190 +10 |
Also, the TODO URL should point to an issue where we are keeping track of that request. We shouldn't be pointing to a PR URL. See ansys/pyansys-geometry#1319 as an example. The first time I implemented the flake8-todo task, I created this longterm tracker. Ideally, each todo should have an independent issue though. |
9d2d0e9
to
af3f010
Compare
@RobPasMue, thanks for the review. I have moved the license header update to another PR as requested. Concerning the issue vs PR links, the most recent of the TODOs dates back to 2022, while others were added in 2021. I searched for related issues but found none. I decided to add PR links instead for context because of the following reasons:
What do you advise? Or maybe I should just follow your footsteps in given example i.e. just create one issue and itemize those TODOs. |
IMO the only way you will actually keep track of pending work is through issues, not PRs. It's hard to keep track of them otherwise. I'd advise to create a longterm tracker and let the @ansys/pydpf-admins dev team handle them whenever they have the chance. |
@RobPasMue thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Related to #296. flake8-todos ruff rule is enabled via this PR