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

Feature fields interfaces #469

Merged
merged 4 commits into from
Sep 14, 2023

Conversation

bebehei
Copy link
Contributor

@bebehei bebehei commented Sep 13, 2023

This adds multiple useful fields to netbox_device_interface.

@bebehei bebehei force-pushed the feature-fields-interfaces branch from 5708396 to 785c34f Compare September 13, 2023 10:04
@fbreckle
Copy link
Collaborator

Hi,

thanks for your MR!

For fields that reference other objects, I prefer the attributes to reflect the type of the referenced object. I.e. parent_id should be parent_device_interface_id and lag should be lag_device_interface_id.

@bebehei bebehei force-pushed the feature-fields-interfaces branch from 785c34f to 5a46cb0 Compare September 13, 2023 11:32
@bebehei
Copy link
Contributor Author

bebehei commented Sep 13, 2023

Fixed the naming as you wished.

Found a bug by myself. (LAG was saved in Parent field). Albeit using the code, did not catch this error. Have to write tests for this.

@bebehei bebehei force-pushed the feature-fields-interfaces branch from 5a46cb0 to b0900a0 Compare September 13, 2023 11:37
@fbreckle fbreckle marked this pull request as draft September 13, 2023 13:29
@bebehei bebehei force-pushed the feature-fields-interfaces branch from b0900a0 to ea4ebe6 Compare September 13, 2023 15:46
@bebehei bebehei force-pushed the feature-fields-interfaces branch from ea4ebe6 to fdd50cf Compare September 13, 2023 15:55
@bebehei
Copy link
Contributor Author

bebehei commented Sep 13, 2023

This took a while. I'm not familiar with golang at all. But finally, it works properly:

  • I've added the tests
  • Official CI is green
  • It also works on my own instance

Please have a look at the tests, if these are missing.

Anything else missing?


Thank you for fixing the ASN stuff and also adding netbox_cable in the meantime!

@bebehei bebehei marked this pull request as ready for review September 13, 2023 16:12
@fbreckle fbreckle merged commit 8bdc117 into e-breuninger:master Sep 14, 2023
@fbreckle
Copy link
Collaborator

fyi: You should not change the docs directly, since they are autogenerated. I made 5911b2f to match your doc updates. Docs can be generated with make docs.

@bebehei
Copy link
Contributor Author

bebehei commented Sep 14, 2023

Oh, I understand. Thank you!

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