-
Notifications
You must be signed in to change notification settings - Fork 138
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
Replace and rewrite "assert" statements in non-test code #175
Comments
Heyyy, I'd like to work on this issue. Can you assign it to me? |
so, assert statements should be removed from all the repos of aboutcode-org., or only from aboutcode-org/scancode-toolkit ? |
Hey, I'd like to work on this issue. Can you assign this to me? And the assert statements are to be removed from all the repos or a specific one? @pombredanne |
Hi @pombredanne , I have started working on this issue. I will replace the assert statements with raise statements in the main code (outside test files). I'll submit a PR soon. Let me know if there are specific repos other than scancode-toolkit where this change is needed. |
@pombredanne I have made a PR on this https://github.com/aboutcode-org/aboutcode-toolkit/pull/574 |
Hi @pombredanne, I would like to work on this issue to replace the assert statements with proper raise exceptions in the main code (outside test files). Could you please assign this issue to me? Let me know if any specific guidelines or repos need to be considered. Thank you! |
@TirthPanchal re:
we do not "assign" issue per se. You are welcome to work on this, but you want to sync up here with others to avoid double, duplicated work! @Dhruv276R re:
As explained above, we cannot "assign" work. You guys need to coordinate and collaborate to avoid stepping on each other.
Rather replaced in all the repos, and only asserts that are not for tests, and only asserts that are for production code that could be ignored from a -O python run. @Gurleen-kansray re:
As said above, all repos, but not in test code: there we use "assert" and pytest does an awesome job assert rewriting job, so we surely do not want to change this. @ONKARBH re:
Same as above: all repos, not the tests, and you need to coordinate here with others |
Hi everyone, I’d like to contribute to this issue by focusing on specific files/modules for replacing assert statements outside the test files. Please let me know how I can best coordinate with others to avoid overlap. Thank you! |
@Dhruv276R re:
This is awesome, but I cannot see the PR at https://github.com/aboutcode-org/aboutcode/pulls/574 ? |
Hi @Dhruv276R, your work is great! However, the PR link seems incorrect. Can you confirm if the PR is in the |
My bad: this is at aboutcode-org/aboutcode-toolkit#574 |
See https://www.reddit.com/r/learnpython/comments/zd217y/is_using_assert_considered_good_practice/
We are using a few asserts in our main code. See https://github.com/search?q=org%3Aaboutcode-org+NOT+path%3A*test*++"assert"++language%3APython&type=code
The text was updated successfully, but these errors were encountered: