Skip to content
This repository has been archived by the owner on Aug 26, 2021. It is now read-only.

Add Code Factor for basic code quality review #8

Closed
Cobular opened this issue May 24, 2019 · 2 comments
Closed

Add Code Factor for basic code quality review #8

Cobular opened this issue May 24, 2019 · 2 comments
Labels
enhancement New feature or request

Comments

@Cobular
Copy link
Owner

Cobular commented May 24, 2019

Hey all,
I'm looking into adding https://www.codefactor.io/ to the project in order to try to catch the most basic errors and keep the quality high.

EDIT: This is (for now) the place to talk about code quality changes that aren't severe enough to merit their own issue. If it will just be 2 or 3 comments, just have it here rather than in a brand new issue.

@Cobular Cobular added the enhancement New feature or request label May 24, 2019
@JosephFKnight
Copy link
Collaborator

That's a good idea. and codefactor actually found an issue that is preventing me from refactoring further: the global EXIT_CODE variable. globals are almost always bad practice and we'll need to get rid of it.

If you explain a bit your rationale for having it in there, I will likely be able to figure out a way to do without it. It's for the CLI mode, yes? So that the program exits with RC 1 if there is a test failure?

@Cobular
Copy link
Owner Author

Cobular commented May 27, 2019

Basically, It's what you noted. I know it's bad practice, and it's sort of a half-assed solution, but I needed a way to set EXIT_CODE at the file level so that it could be exited. See this code snippet:
https://github.com/JakeCover/distest/blob/ce78a862c2e80095423a250edfdd034cb0cbb6e1/distest/__init__.py#L393-L399
I have found that nothing in that function will run after I call client.close(), so I couldn't use sys.exit(1) or a return statement, since that would cause either the exit()/return or close() to not fire. (Does that make sense?) The next place my code runs is right after bot.run() in the following snippet:
https://github.com/JakeCover/distest/blob/ce78a862c2e80095423a250edfdd034cb0cbb6e1/distest/__init__.py#L697-L700
This means that the exit code has to be processed there, if everything else stays the same. I really don't like this solution, as it uses globals, but it's the first thing me and some people on the disord.py server thought of when I brought it up.

EDIT: After writing this, I looked at the PRs and I see that you fixed it in 5bb0c51. That is a much more elegant way to do this! Thank you!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants