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

Renamed HardwareFamily to DeviceFamily. #5418

Merged
merged 2 commits into from
Mar 14, 2024
Merged

Renamed HardwareFamily to DeviceFamily. #5418

merged 2 commits into from
Mar 14, 2024

Conversation

HanlinMiao
Copy link
Contributor

Closes #5352

What's Changed

Pretty Straightforward change here but please do a sanity check to see if I missed any places.

Screenshots

TODO

  • Explanation of Change(s)
  • Added change log fragment(s) (for more information see the documentation)
  • Attached Screenshots, Payload Example
  • Unit, Integration Tests
  • Documentation Updates (when adding/changing features)
  • Example Plugin Updates (when adding/changing features)
  • Outline Remaining Work, Constraints from Design

@@ -1,2 +1,2 @@
Added `HardwareFamily` model class.
Added `hardware_family` field to Device Type model class.
Added `DeviceFamily` model class.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think these are changes for issue #3559, and should be left that way?

Copy link
Contributor

Choose a reason for hiding this comment

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

We don't have a defined standard for maintaining "pre-release" changelog fragments, but I understand the reasoning here - it's potentially confusing to the user when we ship 2.2 for the "added" to refer to things that weren't added as described. On the flip side, it's good for the change fragments to document the actual work done. Perhaps something like the following might be a good compromise?

Suggested change
Added `DeviceFamily` model class.
Added `HardwareFamily` model class. (Renamed before release to `DeviceFamily`.)

nautobot/dcim/filters/__init__.py Outdated Show resolved Hide resolved
nautobot/dcim/filters/__init__.py Outdated Show resolved Hide resolved
@@ -1,2 +1,2 @@
Added `HardwareFamily` model class.
Added `hardware_family` field to Device Type model class.
Added `DeviceFamily` model class.
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't have a defined standard for maintaining "pre-release" changelog fragments, but I understand the reasoning here - it's potentially confusing to the user when we ship 2.2 for the "added" to refer to things that weren't added as described. On the flip side, it's good for the change fragments to document the actual work done. Perhaps something like the following might be a good compromise?

Suggested change
Added `DeviceFamily` model class.
Added `HardwareFamily` model class. (Renamed before release to `DeviceFamily`.)

Comment on lines 18 to 19
router.register("device-families", views.DeviceFamilyViewSet)
router.register("manufacturers", views.ManufacturerViewSet)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit, but maybe swap the order of these two for consistency?

nautobot/dcim/models/devices.py Outdated Show resolved Hide resolved
nautobot/dcim/tables/__init__.py Outdated Show resolved Hide resolved
@@ -227,17 +227,17 @@
),
),
NavMenuItem(
Copy link
Contributor

Choose a reason for hiding this comment

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

Were we going to move Device Families above Manufacturers in the nav as a part of this PR, or is that still up in the air? @lampwins

@HanlinMiao HanlinMiao merged commit a5e6cab into next Mar 14, 2024
17 checks passed
@HanlinMiao HanlinMiao deleted the u/hanlin-#5352 branch March 14, 2024 01:13
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.

None yet

3 participants