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

Refactor Feed & Score Nodes #267

Open
1 of 10 tasks
seanmajorpayne opened this issue Oct 10, 2021 · 0 comments
Open
1 of 10 tasks

Refactor Feed & Score Nodes #267

seanmajorpayne opened this issue Oct 10, 2021 · 0 comments
Assignees

Comments

@seanmajorpayne
Copy link
Member

seanmajorpayne commented Oct 10, 2021

What to refactor?

  • app/scoring/score_nodes.py

Refactor reasons and solutions

  • Score nodes presently use unconventional python naming with variables in ALL_CAPS format,
  • has a few variables that are declared and never used
  • does not use dependency injection as it should.
  • it is only used by the feed but is located in the scoring folder so it should be relocated.
  • Storing the climate feed into the db should not occur in this class.
  • Feed uses some variables in ALL_CAPS, a generalized try/except to catch multiple types of errors,
  • Imports are not in pep8 order & relative imports should be used when possible
  • Both should use Pep 287 reST formatting for docstrings.
  • We're passing a quiz_uuid to a query for a session_uuid in the localized graph function and this is bad!

Tests

  • unit tests
@seanmajorpayne seanmajorpayne added bug Something isn't working refactoring labels Oct 10, 2021
@seanmajorpayne seanmajorpayne self-assigned this Oct 10, 2021
@danmash danmash changed the title Feed & Score Nodes Refactor Refactor Feed & Score Nodes Oct 12, 2022
@danmash danmash removed the bug Something isn't working label Oct 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 📋 Backlog
Development

Successfully merging a pull request may close this issue.

2 participants