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

Code Cleanup #14

Open
mcdonnnj opened this issue Jul 21, 2020 · 1 comment
Open

Code Cleanup #14

mcdonnnj opened this issue Jul 21, 2020 · 1 comment
Assignees

Comments

@mcdonnnj
Copy link
Member

Proposal

The codebase could use another pass to cleanup/adjust to improve future maintainability.

Motivation

The codebase should more strictly follow Python convention to hopefully improve maintainability.

Work

I created a branch and added pylint to the pre-commit configuration with a slightly tweaked configuration file (.pylintrc) to pare down to more "essential" tests. The lint run can be found at https://github.com/cisagov/findcdn/runs/894966344?check_suite_focus=true

@DoctorEww DoctorEww self-assigned this Jul 24, 2020
@Pascal-0x90 Pascal-0x90 mentioned this issue Jul 31, 2020
9 tasks
@S4lt5
Copy link

S4lt5 commented Oct 28, 2022

To add to this, the Chef class probably should not exist.

  • All of the chef's operations are actually operations on DomainPot, or Domain
  • All of the args passed to Chef are then just passed to run_checks and ultimately down the line
  • It's honestly incredibly confusing with the number of functions that just get daisey chained without the reader really understanding why, IMO: global run checks-> chef run checks -> chef grab_cdn -> global chef_executor (huh?) -> detectCDN.cdnCheck() -> all_checks() is a lot more complicated than it could be, to do what amounts to a single operation. The last item in that chain, all_checks already runs on a domain object, so why do I pass the arguments through so many layers that don't really do anything?
  • There's a lot of things that could be broken out into separate files for easier readability

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

No branches or pull requests

4 participants