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

chore: [Policy Assistant] rename Cyclonus to policy-assistant #262

Merged

Conversation

huntergregory
Copy link
Contributor

@huntergregory huntergregory commented Oct 29, 2024

New Binary Name

$ cd cmd/policy-assistant
$ make policy-assistant
$ ./cmd/policy-assistant/policy-assistant analyze --help
...

Used to be:

$ make cyclonus
$ ./cmd/cyclonus/cyclonus analyze --help
...

New Module Name

module sigs.k8s.io/network-policy-api/policy-assistant

Used to be:

module github.com/mattfenwick/cyclonus

Docs Update

  • Adds some READMEs
  • Moves outdated Cyclonus docs/examples to archived/ folders

Add go vet and go fmt checks to GH action

Outputs for go fmt

  • If there's a file that needs formatting:
    • {A62101C5-DD3B-4F0E-B860-070814974115}
  • Otherwise:
    • {183626F3-44A2-4BDB-9B18-49F59C8D3C72}

Signed-off-by: Hunter Gregory <[email protected]>
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Oct 29, 2024
@k8s-ci-robot k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Oct 29, 2024
Copy link

netlify bot commented Oct 29, 2024

Deploy Preview for kubernetes-sigs-network-policy-api ready!

Name Link
🔨 Latest commit ac44f11
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-sigs-network-policy-api/deploys/672c499c8b49870008f0dbc8
😎 Deploy Preview https://deploy-preview-262--kubernetes-sigs-network-policy-api.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@huntergregory huntergregory force-pushed the rename-cyclonus-to-pola branch 13 times, most recently from 6f35941 to 57b8247 Compare October 30, 2024 00:56
@huntergregory huntergregory force-pushed the rename-cyclonus-to-pola branch 2 times, most recently from c8b66d7 to 0a7f048 Compare October 30, 2024 01:02
- name: Run Go Vet
run: |
cd cmd/policy-assistant/
go vet ./...
Copy link
Contributor Author

@huntergregory huntergregory Oct 30, 2024

Choose a reason for hiding this comment

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

This will fail with non-zero exit code if go vet detects a problem

@huntergregory
Copy link
Contributor Author

@aojea or @danwinship could you approve the changes to the Github action? And add a lgtm? Thanks!

@@ -1,3 +1,14 @@
# TODO: create an actual image registry + image
Copy link
Contributor Author

Choose a reason for hiding this comment

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

See #263

@huntergregory
Copy link
Contributor Author

huntergregory commented Oct 30, 2024

fyi @mattfenwick this PR changes the name from Cyclonus to something else like you suggested some time ago

[from #62]
rename to something appropriate for kubernetes-sigs (positive side effect: disambiguates from my personal project)

I'm thinking to change cyclonus binary to pola per thoughts in #255

@danwinship
Copy link
Contributor

/approve
for the workflow changes. It would really be good to get someone else to lgtm the policy agent changes...

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: danwinship, huntergregory

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 Nov 2, 2024
Signed-off-by: Hunter Gregory <[email protected]>
@huntergregory
Copy link
Contributor Author

Renaming binary from cyclonus:

I'd be inclined to name the CLI pola since it seems CLIs are usually short/abbreviated for less typing. Currently the binary is located at cmd/policy-assistant/cmd/cyclonus/cyclonus.

New Binary Name

Fixes #254

$ cd cmd/policy-assistant
$ make pola
$ ./cmd/pola/pola analyze --help
...

Used to be:

$ make cyclonus
$ ./cmd/cyclonus/cyclonus analyze --help
...

@huntergregory
Copy link
Contributor Author

Renaming go module from cyclonus:

idea for new name: sigs.k8s.io/network-policy-api/policy-assistant

NOTE: this would drop the cmd/ directory (pola is located at cmd/policy-assistant/)

New Module Name

Fixes #170

module sigs.k8s.io/network-policy-api/policy-assistant

Used to be:

module github.com/mattfenwick/cyclonus

@tssurya
Copy link
Contributor

tssurya commented Nov 6, 2024

+1 to rename it to policy-assistant from cyclonus @mattfenwick any concerns? (I unfortunately have not reviewed the PR yet just wanted to say I am ok with the name change in response to https://kubernetes.slack.com/archives/C01CWSHQWQJ/p1730829759640629)

@tssurya
Copy link
Contributor

tssurya commented Nov 6, 2024

I am also OK to call the CLI pola and tags pola, its just the dumb me couldn't connect pola as the short abbreviation for policy-assistant ; but I am not sure if that is common in API/CLI usually? We usually always go with long abbreviations and add a short-name alias sorta thing for that long name... but maybe this is OK?

@huntergregory
Copy link
Contributor Author

huntergregory commented Nov 6, 2024

Thanks for the feedback @tssurya . As some context, Gateway API names their CLI for user tooling gwctl. It seems like their project name is "gwctl" as well though?

@huntergregory
Copy link
Contributor Author

I'm convinced that pola isn't a great idea after Surya's feedback above and Mike's feedback offline, where he echoed how it wasn't immediately clear what pola meant. Regarding abbreviations / more typing, Mike also shared how devs frequently write their own aliases like k for kubectl.

I feel like policy-assistant is a decent name for the static analysis tool. And the project is already called policy assistant, so it makes for a nicer user experience if the project name is the same as the binary name. Updating the PR now to replace pola with policy-assistant

@huntergregory huntergregory changed the title chore: [Policy Assistant] rename Cyclonus to pola chore: [Policy Assistant] rename Cyclonus to policy-assistant Nov 7, 2024
@npinaeva
Copy link
Member

npinaeva commented Nov 7, 2024

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 7, 2024
@k8s-ci-robot k8s-ci-robot merged commit 2ce404d into kubernetes-sigs:main Nov 7, 2024
13 checks passed
@tssurya
Copy link
Contributor

tssurya commented Nov 7, 2024

Yea policy-assisstant ++
pola wasn't intuitive enough
Thanks @huntergregory

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
5 participants