Rename example-backend to backend & fix ci#328
Conversation
|
Note Reviews pausedUse the following commands to manage reviews:
WalkthroughRenames “example-backend” to “backend” across CI workflow, Makefile targets, handler metadata, and e2e flag set. Updates docs to reflect new image name/path and bumps example release tags to v0.5.0-rc2 and image reference to v0.5.0. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai ignore |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
backend/http/handler.go (1)
219-229: OAuth2 state decoding uses StdEncoding but it was encoded with URLEncoding.
handleAuthorizeencodes state viabase64.URLEncoding.EncodeToString, buthandleCallbackdecodes withbase64.StdEncoding, which will fail for '-'/'_' characters.Fix the decoder:
- decoded, err := base64.StdEncoding.DecodeString(state) + decoded, err := base64.URLEncoding.DecodeString(state)Also applies to: 251-260
docs/content/setup/local-setup-with-kind.md (1)
231-241: Fix endpoint path: use /exports (plural).Backend serves GET on “/exports” (and “/clusters/{cluster}/exports”), not “/export”. The current command will 404.
Apply:
- $ kubectl bind http://${BACKEND_HOST_IP}:8080/export + $ kubectl bind http://${BACKEND_HOST_IP}:8080/exports
🧹 Nitpick comments (5)
docs/content/contributing/guides/publishing-a-new-kube-bind-release.md (2)
32-33: Keep tag examples consistent across sections.You updated examples to
v0.5.0-rc2here, but the “Generate Release Notes” section still usesTAG=v0.5.0, which may confuse readers.Suggested tweak:
- TAG=v0.5.0 + TAG=v0.5.0-rc2Optionally add a note at the top: “Set TAG to the exact version you’re releasing (e.g., v0.5.0-rc2 or v0.5.0).”
Also applies to: 40-42, 48-50, 56-58
36-44: Minor: list indentation nit picked by markdownlint.Indentation around item “3. Tag the sdk module” is off versus peers (MD005). Align list markers’ indentation for consistent rendering.
docs/content/setup/local-setup-with-kind.md (1)
177-183: Correct port note: 30080 vs 30800.You expose NodePort 30080 earlier; the note says 30800. Align the text.
- # Kind to expose 30800 at host's 8080. + # Kind to expose 30080 at host's 8080.Makefile (2)
387-393: Rename applied correctly for local ko build.Message and build path now target ./cmd/backend as intended. Consider DRY-ing common ko flags (-B, --local, -t $(REV)) into a KO_FLAGS var to avoid drift across images.
396-401: Add a minimal “clean” target to satisfy checkmake and developer ergonomics.Static analysis flags a missing clean target. Suggest adding a basic one.
+.PHONY: clean +clean: + rm -rf $(GOBIN_DIR) .kcp coverage.txt
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/workflows/image.yaml(1 hunks)Makefile(1 hunks)backend/http/handler.go(1 hunks)docs/content/contributing/guides/publishing-a-new-kube-bind-release.md(1 hunks)docs/content/setup/local-setup-with-kind.md(2 hunks)test/e2e/framework/backend.go(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
docs/content/contributing/guides/publishing-a-new-kube-bind-release.md
36-36: Inconsistent indentation for list items at the same level
Expected: 0; Actual: 1
(MD005, list-indent)
🪛 checkmake (0.2.2)
Makefile
[warning] 396-396: Missing required phony target "clean"
(minphony)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: verify
- GitHub Check: go-test-e2e
- GitHub Check: lint
- GitHub Check: go-test
🔇 Additional comments (3)
test/e2e/framework/backend.go (1)
73-73: LGTM: FlagSet renamed to “backend”.Matches the repo-wide rename and keeps test output consistent.
docs/content/setup/local-setup-with-kind.md (1)
6-7: LGTM on rename + image/entrypoint updates.References now point to kube-bind/backend and /ko-app/backend correctly.
Also applies to: 164-167
.github/workflows/image.yaml (1)
50-57: LGTM: backend image publish step targets ./cmd/backend and correct repo.Matches the rename and KO_DOCKER_REPO layout.
| Version: ver, | ||
| ProviderPrettyName: "example-backend", | ||
| ProviderPrettyName: "backend", | ||
| AuthenticationMethods: []kubebindv1alpha2.AuthenticationMethod{ |
There was a problem hiding this comment.
Do not hard-code ProviderPrettyName; honor the configured flag.
This ignores NewHandler’s providerPrettyName (and the CLI flag like --pretty-name), reducing configurability.
Apply this to use the configured value with a sane default:
@@
- provider := &kubebindv1alpha2.BindingProvider{
+ // prefer configured pretty name; default to "backend"
+ prettyName := h.providerPrettyName
+ if prettyName == "" {
+ prettyName = "backend"
+ }
+ provider := &kubebindv1alpha2.BindingProvider{
@@
- ProviderPrettyName: "backend",
+ ProviderPrettyName: prettyName,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Version: ver, | |
| ProviderPrettyName: "example-backend", | |
| ProviderPrettyName: "backend", | |
| AuthenticationMethods: []kubebindv1alpha2.AuthenticationMethod{ | |
| // prefer configured pretty name; default to "backend" | |
| prettyName := h.providerPrettyName | |
| if prettyName == "" { | |
| prettyName = "backend" | |
| } | |
| provider := &kubebindv1alpha2.BindingProvider{ | |
| Version: ver, | |
| ProviderPrettyName: prettyName, | |
| AuthenticationMethods: []kubebindv1alpha2.AuthenticationMethod{ | |
| // … | |
| }, | |
| } |
🤖 Prompt for AI Agents
In backend/http/handler.go around lines 152-154, the ProviderPrettyName is
hard-coded to "backend"; replace that literal with the configured
providerPrettyName value from the handler (or the NewHandler parameter) and fall
back to a sensible default if empty (e.g., use the existing Provider or ver
value). Ensure the struct uses something like ProviderPrettyName:
h.providerPrettyName (or providerPrettyName) with a conditional default when
constructing the object so the CLI flag --pretty-name is honored.
✅ Actions performedReviews paused. |
aa1c558 to
506ddb9
Compare
mjudeikis
changed the title
2 participants
Summary
Some renames were still missing. Rename example-backend to backend and fix CI, docs.
What Type of PR Is This?
/kind cleanup
/kind documentation
Related Issue(s)
Fixes #
Release Notes
Summary by CodeRabbit
New Features
Chores
Documentation
Tests
Refactor