-
Notifications
You must be signed in to change notification settings - Fork 63
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
Migrationgraphdoc #437
Migrationgraphdoc #437
Conversation
@munrojm Hi Jason! Do I need to add a test to this? If so, is there a good example test file that I can go off of? If not, I believe it's ready for review! Thanks! |
Hey @hmlli! Thanks for the PR! Left one comment. As for tests, you should add one to test the class method attached to the doc model. Here are the tests for the electrode-related document models for example: https://github.com/materialsproject/emmet/blob/main/tests/emmet-core/test_electrodes.py. |
hi @munrojm, I've finished adding the test, this PR should be ready for review. Thanks :) |
Codecov Report
@@ Coverage Diff @@
## main #437 +/- ##
=======================================
Coverage 97.67% 97.67%
=======================================
Files 114 115 +1
Lines 23837 23856 +19
=======================================
+ Hits 23283 23302 +19
Misses 554 554
Continue to review full report at Codecov.
|
@munrojm I have merged changes from |
@hmlli sounds good! Last thing is just addressing the comment I left above regarding the try-except block. After that is changed, I will merge. |
Start with a description of this PR. Then edit the list below to the items that make sense for your PR scope, and check off the boxes as you go!
Contributor Checklist