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.

Sign up for GitHub

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

Jump to bottom

Fix CRD group name not following conventions #68

Closed
wants to merge 4 commits into from
Closed

Fix CRD group name not following conventions #68

wants to merge 4 commits into from

Conversation

faebr
Copy link
Contributor

@faebr faebr commented Sep 20, 2024 •
edited
Loading

  • Changed domain to netboxlabs.com
  • Changed group to ipam
  • Adapted api paths and crd group to convention: ipam.netboxlabs.com
  • Thought about changing pkg/netbox to pkg/ipam but after starting the change, realized that there is for example ipam.go in models and lots of names containing netbox in this pkg so probably confusing to change, so currently left it like this. Any thoughts on this?

@faebr faebr linked an issue Sep 20, 2024 that may be closed by this pull request
CRD group name not following conventions #66
Closed
2 tasks
@henrybear327
Copy link
Collaborator

Note: merge only after 0.0.1-alpha.1 is released!

@henrybear327 henrybear327 force-pushed the 66-crd-group-name-not-following-conventions branch from 5d84484 to 0b7be53 Compare September 24, 2024 06:53
@jstudler
Copy link
Contributor

Why netboxlabs.com and not netbox.dev? NetBox Labs is the company and not the community, this project is hosted on the community.

@faebr
Copy link
Contributor Author

faebr commented Sep 24, 2024

Why netboxlabs.com and not netbox.dev? NetBox Labs is the company and not the community, this project is hosted on the community.

@jstudler, the netbox.dev domain is not mentioned when looking at the github, but only netboxlabs.com. Also @rboucher-me mentioned this would be their preference when asked. Those were the 2 main reasons.

@henrybear327 henrybear327 mentioned this pull request Sep 24, 2024
Merged
@henrybear327
Copy link
Collaborator

henrybear327 commented Sep 24, 2024 •
edited
Loading

We will make another release after this is merged, as this is a breaking change

@henrybear327 henrybear327 changed the title fix: 66-crd-group-name-not-following-conventions Fix CRD group name not following conventions Sep 25, 2024
@henrybear327 henrybear327 force-pushed the 66-crd-group-name-not-following-conventions branch from 0b7be53 to 9a2cd1f Compare September 25, 2024 09:28
@henrybear327
Copy link
Collaborator

After some discussions on Slack, we decided that we would prefer to stick with the netbox.dev domain.

The main reasons are

  • the NetBox-operator is built for NetBox, thus, netbox.dev is a clearer indication of what this project is built for
  • the NetBox-Operator is not related to netboxlabs.com

henrybear327
henrybear327 previously approved these changes Sep 26, 2024
View reviewed changes
Copy link
Collaborator

@henrybear327 henrybear327 left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks @faebr

@henrybear327
Copy link
Collaborator

@faebr please resolve the rebase conflict :( Thanks!

@henrybear327
Copy link
Collaborator

henrybear327 commented Sep 27, 2024 •
edited
Loading

The PR base has been changed to #74. More conflicts to resolve :( @faebr

@henrybear327 henrybear327 changed the base branch from main to feature/support-ipv6-reservation September 27, 2024 08:44
@bruelea bruelea force-pushed the feature/support-ipv6-reservation branch 3 times, most recently from eea770e to 593568c Compare September 30, 2024 13:04
Base automatically changed from feature/support-ipv6-reservation to main September 30, 2024 13:52
@henrybear327 henrybear327 requested review from henrybear327 and removed request for henrybear327 October 3, 2024 09:48
@henrybear327 henrybear327 dismissed their stale review October 3, 2024 09:49

New discussion requiring new changes

@henrybear327 henrybear327 removed their request for review October 3, 2024 09:49
@henrybear327
Copy link
Collaborator

@MaIT-HgA do you want to rebase this PR and make it ready for merging :)

@faebr
changed domain to netboxlabs.com and group to ipam. Adapted all group…
d8884b8 
…/domain occurences to be consistent with a fesh kubebuilder project
@MaIT-HgA MaIT-HgA force-pushed the 66-crd-group-name-not-following-conventions branch from 8e1d5a2 to a206a4e Compare October 9, 2024 05:39
@MaIT-HgA
Copy link
Contributor

MaIT-HgA commented Oct 9, 2024

@MaIT-HgA do you want to rebase this PR and make it ready for merging :)

done :)

@MaIT-HgA MaIT-HgA requested a review from henrybear327 October 9, 2024 05:40
@henrybear327
Copy link
Collaborator

henrybear327 commented Oct 9, 2024 •
edited
Loading

Thanks @MaIT-HgA for rebasing it!

As discussed during the meeting, we have decided to postpone the merging of this PR, as this would require migration to be done for the existing CRs and it's currently de-scoped from our development sprint.

@henrybear327 henrybear327 marked this pull request as draft October 9, 2024 07:30
@faebr
Copy link
Contributor Author

faebr commented Oct 29, 2024 •
edited
Loading

This PR will be closed, an easier solution has been found. We will remove the crd group from the PROJECT file and thus the already created crd's are alligned again. New crd's can be added without a group or inside a new group after enabling multi-group support in the said file.

@faebr faebr closed this Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jstudler jstudler jstudler left review comments

@alexandernorth alexandernorth Awaiting requested review from alexandernorth

@bruelea bruelea Awaiting requested review from bruelea

@henrybear327 henrybear327 Awaiting requested review from henrybear327

Assignees

@faebr faebr

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

CRD group name not following conventions
4 participants
@faebr @henrybear327 @jstudler @MaIT-HgA