-
Notifications
You must be signed in to change notification settings - Fork 4
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
Allow useof HFID to create a related node on generic relationship #292
Conversation
Codecov ReportAttention: Patch coverage is
@@ Coverage Diff @@
## stable #292 +/- ##
==========================================
+ Coverage 70.32% 70.35% +0.03%
==========================================
Files 81 81
Lines 7507 7510 +3
Branches 1464 1465 +1
==========================================
+ Hits 5279 5284 +5
+ Misses 1846 1842 -4
- Partials 382 384 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
|
54e0880
to
c9cc008
Compare
infrahub_sdk/node.py
Outdated
@@ -255,6 +256,9 @@ def _generate_input_data(self, allocate_from_pool: bool = False) -> dict[str, An | |||
data["id"] = self.id | |||
elif self.hfid is not None: | |||
data["hfid"] = self.hfid | |||
if self._kind is not None: | |||
# We might need `kind` for https://github.com/opsmill/infrahub/issues/4649 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it is sustainable to add comments referencing GH issues
if we're worried about someone removing this line and breaking some piece of functionality, then we should have a unit test that covers it
for me, this seems like a simple enough change that a comment is not necessary at all
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree referencing issues should be either within dedicated test or for bigger changes, I'll remove this one
@@ -80,6 +80,8 @@ def schema_cat(self) -> NodeSchema: | |||
namespace=NAMESPACE, | |||
include_in_menu=True, | |||
inherit_from=[TESTING_ANIMAL], | |||
# Different hfid than Animal one, for testing https://github.com/opsmill/infrahub-sdk-python/issues/277 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd say this comment should be in the test that uses it and the test can have an assert cat_luna.hfid == ...
if you're concerned about someone changing this and invalidating the test
but I don't think we can add a comment every time we alter a fixture
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense!
c9cc008
to
6901418
Compare
No description provided.