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

Gets Baseline Test Working With Current State of Codebase #91

Merged
merged 6 commits into from
Aug 22, 2024

Conversation

jmwright
Copy link
Contributor

This is done in preparation for moving to assembly CAD and rendering.

@jmwright
Copy link
Contributor Author

@julianstirling I'll work on fixing the pylint errors in the shelf.py module.

@julianstirling
Copy link
Collaborator

@julianstirling I'll work on fixing the pylint errors in the shelf.py module.

Thanks!

@jmwright
Copy link
Contributor Author

@julianstirling Everything is green now, and the new test is running in CI. However, you may take issue with my removal of the TODO entry from the pylint RC file. I also fixed some other things even after pylint was passing to give us some breathing-room before dropping below the 9.75 score threshold. Commenting some of the Shelf methods that are not implemented yet to pass pylint is hacky, but I will be working on them next and so I am not too worried about it.

In the interest of doing smaller PRs that handle one thing at a time, I'll take this out of draft so that it can be fully reviewed. Once we iterate and it is merged, I'll move on to assemblies and renders.

@jmwright jmwright marked this pull request as ready for review August 21, 2024 15:31
@julianstirling
Copy link
Collaborator

I am wondering if rather than commenting out the currently unfinished functions to make pylint happen we instead add:

 #pylint: disable=unused-argument

into the functions just before the return NotImplemented

This will silence the error but let us have the methods in place ready?

@julianstirling
Copy link
Collaborator

I'm happy with pulling the TODOs out of the RC file.

@jmwright
Copy link
Contributor Author

I am wondering if rather than commenting out the currently unfinished functions to make pylint happen we instead add:

 #pylint: disable=unused-argument

into the functions just before the return NotImplemented

This will silence the error but let us have the methods in place ready?

Ok, that has been implemented in the latest commit.

Copy link
Collaborator

@julianstirling julianstirling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this. Great to have tests, docstrings, and to have the linter passing again.

@julianstirling julianstirling merged commit 70db70c into master Aug 22, 2024
5 checks passed
@jmwright jmwright deleted the create-shelves-from-device-id-jmw branch August 22, 2024 18:45
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

Successfully merging this pull request may close these issues.

2 participants