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

Standardize on naming for ContentType fields #4711

Closed
jeremystretch opened this issue Jun 3, 2020 · 2 comments
Closed

Standardize on naming for ContentType fields #4711

jeremystretch opened this issue Jun 3, 2020 · 2 comments
Labels
status: accepted This issue has been accepted for implementation type: housekeeping Changes to the application which do not directly impact the end user
Milestone

Comments

@jeremystretch
Copy link
Member

jeremystretch commented Jun 3, 2020

Proposed Changes

Update models in NetBox to use a standardized name for ForeignKey fields to the ContentType model. The name object_type is most appropriate IMO. Applicable fields include:

  • dcim.Cable
    • termination_a_type
    • termination_b_type
  • extras.CustomField
    • obj_type
  • extras.CustomFieldValue
    • obj_type
  • extras.CustomLink
    • content_type
  • extras.ExportTemplate
    • content_type
  • extras.Graph
    • type
  • extras.ImageAttachment
    • content_type
  • extras.ObjectChange
    • changed_object_type
    • related_object_type
  • extras.Webhook
    • obj_type
  • users.ObjectPermission
    • content_types (v2.9)

The proposal here is to rename fields named type, obj_type, or content_type to object_type. The Cable and ObjectChange models likely need no changes.

Justification

  • Standardizing on a common name readily conveys the purpose of the field.
  • object_type is more accurate than content_type and more apparent than obj_type. (type alone should be avoided due to its status as a reserved word in Python.)
@jeremystretch jeremystretch added type: housekeeping Changes to the application which do not directly impact the end user status: under review Further discussion is needed to determine this issue's scope and/or implementation labels Jun 3, 2020
@jeremystretch jeremystretch added status: accepted This issue has been accepted for implementation and removed status: under review Further discussion is needed to determine this issue's scope and/or implementation labels Jul 24, 2020
@jeremystretch jeremystretch added this to the v2.10 milestone Aug 21, 2020
@DanSheps
Copy link
Member

My view:

Standardization is good, where we can get away with it. I know there are going to be places where we can (ObjectPermissions) but we can get close.

I would be worried however about how many instances we call the object_type (or the others) directly. I don't think there is many but could be wrong.

@jeremystretch jeremystretch self-assigned this Oct 13, 2020
@jeremystretch
Copy link
Member Author

Thought about this some more. Seems fine to just change obj_type on CustomField and Webhook to content_types, since these are ManyToManyFields. We can leave the rest as they are.

@jeremystretch jeremystretch added needs milestone Awaiting prioritization for inclusion with a future NetBox release and removed status: accepted This issue has been accepted for implementation labels Oct 14, 2020
@jeremystretch jeremystretch removed their assignment Oct 14, 2020
@jeremystretch jeremystretch removed this from the v2.10 milestone Oct 14, 2020
@jeremystretch jeremystretch added status: accepted This issue has been accepted for implementation and removed needs milestone Awaiting prioritization for inclusion with a future NetBox release labels Dec 2, 2020
@jeremystretch jeremystretch added this to the v2.10 milestone Dec 2, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: accepted This issue has been accepted for implementation type: housekeeping Changes to the application which do not directly impact the end user
Projects
None yet
Development

No branches or pull requests

2 participants