Skip to content

KueueViz to prepare for production suitable: Various renaming #4753

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
Merged
merged 3 commits into from
Mar 24, 2025
Merged

KueueViz to prepare for production suitable: Various renaming #4753

merged 3 commits into from
Mar 24, 2025
+137 −129

Conversation

akram
Copy link
Contributor

@akram akram commented Mar 22, 2025

What type of PR is this?

/kind cleanup

Move KueueViz from experimental to non experimental.
Operate various renaming for consistency reasons: KueueViz is now written KueueViz without hyphen or space or camel cases.
A few exceptions:

  • all image names and code path are kueueviz
  • helm chart value is still kueueViz for consistency with helm parameters

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?

ATTENTION: Many renaming changes to prepare KueueViz to be suitable for production:
- KueueViz is now mentioned KueueViz for its name, label and description
- Path names and image names are now kueueviz (without hyphen) for consistency and naming restrictions
- Helm chart values `.Values.kueueViz` remains unchanged as all parameters in helm chart values starts with lower case.

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Mar 22, 2025
@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Mar 22, 2025
@netlify Netlify
Copy link

netlify bot commented Mar 22, 2025 •
edited
Loading

✅ Deploy Preview for kubernetes-sigs-kueue canceled.

Name Link
🔨 Latest commit 22648c2
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-sigs-kueue/deploys/67e125800be2fd0008c5a1cc

@akram akram changed the title Kueueviz to production: Various renaming KueueViz to prepare for production suitable: Various renaming Mar 22, 2025
@akram akram force-pushed the kueueviz-to-production branch 2 times, most recently from 79a66e1 to 8823b26 Compare March 22, 2025 17:12
@akram
Copy link
Contributor Author

akram commented Mar 22, 2025

@kannon92 if you can PTAL ?

kannon92
kannon92 reviewed Mar 23, 2025
View reviewed changes
.github/dependabot.yml
- package-ecosystem: "gomod"
directories:
- "/cmd/experimental/kueue-viz/backend"
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems weird to put this under cmd.

Should we give a dedicated folder for kueue-viz at top level?

WDYT @mimowo @tenzen-y?

Copy link
Contributor

Choose a reason for hiding this comment

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

Keeping the main entrypoints for tooling seems to be the practice in core k8s, ptal: https://github.com/kubernetes/kubernetes/tree/master/cmd, eg kubemark or kubeadm, but many more.

It is also consistent with kueuectl in Kueue.

However, I would be ok to put the main body of the code under top-level kueueviz.

Seems like kubectl only has minimal main.go under cmd/kubectl: https://github.com/kubernetes/kubernetes/tree/master/cmd/kubectl

Copy link
Contributor

Choose a reason for hiding this comment

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

cc @tenzen-y any opinion?

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 this can be done in a follow up actually. I think the PR in the current form already archives the goal of moving out of experimental. and places the code next to kueuectl, so IMO this is a natural place, also given k8s convention, but Im open to move again if we conclude on a better place.

@mimowo
Copy link
Contributor

mimowo commented Mar 24, 2025

LGTM overall, just two remaining open comments.

@akram
Copy link
Contributor Author

akram commented Mar 24, 2025

/kind dashboard

@k8s-ci-robot k8s-ci-robot added kind/dashboard Denotes a PR that is related to the built-in dashboard needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Mar 24, 2025
@akram akram marked this pull request as draft March 24, 2025 08:51
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 24, 2025
@akram akram force-pushed the kueueviz-to-production branch from 8823b26 to 9bfe747 Compare March 24, 2025 09:01
@akram akram marked this pull request as ready for review March 24, 2025 09:01
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 24, 2025
akram added 2 commits March 24, 2025 10:07
@akram
Moving kueue-viz to cmd/kueueviz
114457a
@akram
Renaming kueue-viz to kueueviz and use of KueueViz globally

Verified

This commit was signed with the committer’s verified signature.
politas Myk
GPG key ID: 47938AAEAD85818F
Verified
Learn about vigilant mode
a4ddbf8
@akram akram force-pushed the kueueviz-to-production branch from 9bfe747 to 7d4a56c Compare March 24, 2025 09:07
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 24, 2025
@mimowo
Copy link
Contributor

mimowo commented Mar 24, 2025

/lgtm
/approve
Thanks!

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 24, 2025
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 6ebe6270dd615f113b2e48da9848de998ecabf62

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: akram, mimowo

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 24, 2025
@akram
KueueViz experimental cleanup
Loading
Loading status checks…
22648c2
@akram akram force-pushed the kueueviz-to-production branch from 7d4a56c to 22648c2 Compare March 24, 2025 09:27
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 24, 2025
@k8s-ci-robot k8s-ci-robot requested a review from mimowo March 24, 2025 09:27
@mimowo
Copy link
Contributor

mimowo commented Mar 24, 2025

/hold for fixing

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 24, 2025
@mimowo
Copy link
Contributor

mimowo commented Mar 24, 2025

/unhold
ok I see the new changes
/lgtm

@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Mar 24, 2025
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 4a9306af7489db465424d6470da870fb0c9da3a5

@k8s-ci-robot k8s-ci-robot merged commit 306f744 into kubernetes-sigs:main Mar 24, 2025
21 checks passed
@k8s-ci-robot k8s-ci-robot added this to the v0.12 milestone Mar 24, 2025
@mbobrovskyi mbobrovskyi mentioned this pull request Mar 24, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tenzen-y tenzen-y

@kannon92 kannon92

@mbobrovskyi mbobrovskyi

@mimowo mimowo

Assignees

@mimowo mimowo

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. kind/dashboard Denotes a PR that is related to the built-in dashboard lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
v0.12
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@akram @mimowo @k8s-ci-robot @kannon92