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

Fixes: #18433 - Fix missing is_primary property on MACAddress model #18444

Merged
merged 3 commits into from
Jan 27, 2025

Conversation

DanSheps
Copy link
Member

Fixes: #18433 - Add missing is_primary property on MACAddress model

  • Added missing is_primary property method on MACAddress model

@DanSheps DanSheps requested a review from bctiemann January 21, 2025 04:23
@DanSheps DanSheps self-assigned this Jan 21, 2025
@bctiemann
Copy link
Contributor

I had is_primary on the model as a concrete field initially, then changed it to primary_for_interface field on the related model during development/review. I guess the template still had the vestigial mention. I'm not sure why we didn't put it back in as a property, but I seem to remember wanting to but there was some argument against it?

Approved for the moment, but there might be some discussion to be had.

bctiemann
bctiemann previously approved these changes Jan 21, 2025
@DanSheps
Copy link
Member Author

I had is_primary on the model as a concrete field initially, then changed it to primary_for_interface field on the related model during development/review. I guess the template still had the vestigial mention. I'm not sure why we didn't put it back in as a property, but I seem to remember wanting to but there was some argument against it?

I think is_primary as a "Boolean" makes sense. If it was is_primary as the reverse relation it would be kind of weird IMO.

bctiemann
bctiemann previously approved these changes Jan 24, 2025
@bctiemann
Copy link
Contributor

@DanSheps please fix merge conflict

@bctiemann bctiemann merged commit cf64f3c into main Jan 27, 2025
6 checks passed
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.

MAC address not shown as "primary for interface" in MAC address detail view
3 participants