feat: add github templates, tooling, and general developer improvements #245
Conversation
|
Warning Rate limit exceeded@RoseSecurity has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 11 minutes and 27 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (4)
WalkthroughAdds repository-wide editor and ignore configs, introduces GitHub issue and PR templates, adds Brew dependencies, provides a Makefile with Helm chart workflows, creates contribution guidelines, and updates README by removing contributor/versioning/changelog sections. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant Make as Makefile
participant AH as ah (ArtifactHub lint)
participant CT as ct (Chart Testing)
participant Helm as helm
Dev->>Make: make test
Note over Make: test depends on lint, template, dry-run
Make->>Make: lint
Make->>AH: ah lint charts/n8n
AH-->>Make: results
Make->>CT: ct lint --chart-dir charts/n8n
CT-->>Make: results
Make->>Make: template
Make->>Helm: helm template charts/n8n
Helm-->>Make: rendered manifests
Make->>Make: dry-run
Make->>Helm: helm install --dry-run --debug
Helm-->>Make: simulated release output
Make-->>Dev: test completed
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Pre-merge checks (3 passed)✅ 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
README.md (5)
40-40: Typo in heading.“Configurating” → “Configuring”; prefer lowercase “n8n” for consistency.
-## Configurating N8n via Values and Environment Variables +## Configuring n8n via values and environment variables
155-155: Incorrect env var name (case).Use DB_POSTGRESDB_PASSWORD (uppercase) to match n8n’s expected variable.
- # The YAML entry db.postgresdb.password: my_secret is transformed DB_POSTGRESDB_password=bXlfc2VjcmV0 + # The YAML entry db.postgresdb.password: my_secret is transformed DB_POSTGRESDB_PASSWORD=bXlfc2VjcmV0
168-173: Wrong env var key in example.It should be DB_POSTGRESDB_DATABASE, not N8N_DB_POSTGRESDB_NAME.
- # N8N_DB_POSTGRESDB_NAME: + # DB_POSTGRESDB_DATABASE: valueFrom: secretKeyRef: name: db-app key: dbname
799-805: Property name typo.Use
scaling.enabled(with “d”).-To enable this mode within this helm chart, you simply should -set `scaling.enable` to true. +To enable this mode within this Helm chart, set `scaling.enabled` to true.
807-810: Path reference likely incorrect.
scaling.worker.replicaCountdoesn’t exist in values; replicas belong underworker.replicaCount.-You can define to spawn more workers, by set scaling.worker.replicaCount to a higher -number. +To spawn more workers, set `worker.replicaCount` to a higher number.
🧹 Nitpick comments (11)
Brewfile (1)
1-2: Optional: include common Helm tooling in Brewfile.Consider adding these to streamline local workflows:
- helm (CLI)
- chart-releaser (for release automation)
- yamllint, yq (YAML hygiene)
Example:
brew "artifacthub/cmd/ah" brew "chart-testing" +brew "helm" +brew "chart-releaser" +brew "yamllint" +brew "yq"(chart-releaser is available via Homebrew.) (formulae.brew.sh)
README.md (2)
62-62: Minor: remove stray space in Markdown link.The extra space can break some renderers.
-Consult the [n8n Environment Variables Documentation]( https://docs.n8n.io/hosting/configuration/environment-variables/) +Consult the [n8n Environment Variables Documentation](https://docs.n8n.io/hosting/configuration/environment-variables/)
71-73: Optional: version placeholder.Swap hard-coded “1.0.0” for a placeholder to reduce drift.
-helm install my-n8n oci://8gears.container-registry.com/library/n8n --version 1.0.0 +helm install my-n8n oci://8gears.container-registry.com/library/n8n --version <CHART_VERSION>CONTRIBUTING.md (1)
3-10: Tighten steps with explicit commands.Suggest showing the exact commands contributors should run.
-4. Run Chart-Testing `make lint` -5. Install the charts and examples locally to see if they work +4. Run linting: `make lint` (runs Artifact Hub `ah lint` and Chart-Testing) +5. Validate templates: `make template` +6. Install locally for a smoke test: `make install` (or `make upgrade`) -6. Submit your PR -7. The maintainers create a new release in GitHub using the chart version number as the tag and title. +7. Submit your PR +8. Maintainers create a new GitHub release using the chart version as the tag and title.If you want, I can align these with the actual Makefile targets.
.editorconfig (1)
27-30: Optional: add shell script stanza.Helm repos often include helper scripts; consider consistent 2-space indent for sh.
[Makefile] indent_style = tab indent_size = 4 +[*.sh] +indent_style = space +indent_size = 2Makefile (6)
2-3: Make variables overridable with sensible defaultsUse ?= so env/CLI overrides stick cleanly.
-CHART_DIR = charts/n8n -CHART_NAME = n8n +CHART_DIR ?= charts/n8n +CHART_NAME ?= n8n
5-10: Add analltarget to satisfy tooling and provide a conventional entrypointAddresses checkmake warning; alias to help (or
testif you prefer)..PHONY: help help: ## Show this help message @echo "Available targets:" @grep -E '^[a-zA-Z_-]+:.*?## .*$$' $(MAKEFILE_LIST) | sort | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-20s\033[0m %s\n", $$1, $$2}' +.PHONY: all +all: help ## Default target
30-34: Make dry-run idempotent and namespace-awareUse
upgrade --installto avoid failures when a release already exists; thread namespace creation.dry-run: ## Perform a dry-run installation @echo "Performing dry-run installation..." - helm install $(CHART_NAME) $(CHART_DIR) --dry-run --debug + helm upgrade --install $(RELEASE) $(CHART_DIR) --namespace $(NAMESPACE) --create-namespace --dry-run --debugAdd near the variables (outside this hunk):
RELEASE ?= $(CHART_NAME) NAMESPACE ?= default
36-45: Propagate namespace and add sensible wait/timeoutHelm ops should be explicit about namespace and not return before resources are ready.
install: ## Install the chart locally @echo "Installing chart locally..." - helm install $(CHART_NAME) $(CHART_DIR) + helm install $(RELEASE) $(CHART_DIR) --namespace $(NAMESPACE) --create-namespace --wait --timeout 5m upgrade: ## Upgrade the installed chart @echo "Upgrading chart..." - helm upgrade $(CHART_NAME) $(CHART_DIR) + helm upgrade $(RELEASE) $(CHART_DIR) --namespace $(NAMESPACE) --wait --timeout 5m
47-50: Scope uninstall to the namespace and avoid hangingPass namespace and a timeout for predictable teardown.
uninstall: ## Uninstall the chart @echo "Uninstalling chart..." - helm uninstall $(CHART_NAME) + helm uninstall $(RELEASE) --namespace $(NAMESPACE) --timeout 5m
55-57: Clean packaged charts in chart dir as wellCovers both root and chart-local packages.
clean: ## Clean up any generated files @echo "Cleaning up..." - @rm -f *.tgz + @rm -f *.tgz $(CHART_DIR)/*.tgz
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
.editorconfig(1 hunks).github/ISSUE_TEMPLATE/bug_report.yml(1 hunks).github/ISSUE_TEMPLATE/feature_request.yml(1 hunks).github/PULL_REQUEST_TEMPLATE.md(1 hunks).gitignore(1 hunks)Brewfile(1 hunks)CONTRIBUTING.md(1 hunks)Makefile(1 hunks)README.md(1 hunks)
🧰 Additional context used
🪛 checkmake (0.2.2)
Makefile
[warning] 54-54: Missing required phony target "all"
(minphony)
🔇 Additional comments (5)
Brewfile (1)
1-2: Formulas look correct (ah + chart-testing).Verified install names: artifacthub/cmd/ah and chart-testing are valid Homebrew formulas. (artifacthub.io)
.gitignore (1)
7-7: LGTM: ignore macOS metadata.Adding .DS_Store is standard and harmless.
.github/PULL_REQUEST_TEMPLATE.md (1)
25-27: ct lint path verified — no change requiredFound charts/n8n/Chart.yaml; the ct lint command in .github/PULL_REQUEST_TEMPLATE.md is correct.
.github/ISSUE_TEMPLATE/feature_request.yml (1)
1-36: Template reads well.Good prompts and required fields to reduce back-and-forth.
.editorconfig (1)
1-35: Solid baseline.Rules fit a Helm chart repo; Markdown exemption for trailing whitespace is thoughtful.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
@RoseSecurity thank you this looks good.I am really not a big fan of makefiles, but given the little complexity here, this should do it. |
2 participants
Why
Note
I also support removing the GitHub Action that checks for conventional commits, as I believe it hinders community contributions. However, I didn’t want to add that to this discussion without first consulting the team.
What
CONTRIBUTING.mdfile outlining the contribution process, chart versioning schema, and changelog location. The contribution guide was removed fromREADME.mdto avoid duplication..github/PULL_REQUEST_TEMPLATE.md) to ensure PRs are well-documented and follow project standards..github/ISSUE_TEMPLATE/to standardize and streamline issue submissions.Makefilewith common targets for linting, templating, installation, and testing to simplify local development and CI processes..editorconfigfile to enforce consistent code formatting across different editors and file types.Brewfilelisting required developer tools for easy setup of the development environment.Summary by CodeRabbit
Chores
Documentation