feat(settings): configurable branch name prefix#2708
Open
KarloAldrete wants to merge 2 commits into
Open
Conversation
The branch name for new tasks was hardcoded with the `posthog-code/` prefix. This adds a "Branch name prefix" setting (Settings -> Worktrees) so it can be changed or removed. - New `branchPrefix` setting, defaulting to the existing `posthog-code/`. - `deriveBranchName` / `suggestBranchName` take an optional `prefix` (default unchanged), so existing callers are unaffected. - `getSuggestedBranchName` reads the setting, so both worktree and cloud branch suggestions honor it. - The prefix is validated before saving (rejects invalid git names and a leading dash); empty means no prefix. Closes PostHog#1632 Generated-By: PostHog Code Task-Id: 8e9f327d-84b1-4608-9f48-c3038dbf87ca
Contributor
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
packages/core/src/git-interaction/branchName.test.ts:114-150
**Prefer parameterised tests for `validateBranchPrefix`**
The five separate `it` blocks under `validateBranchPrefix` all call the same function with different inputs; this is exactly the pattern the team prefers to express as `it.each`. A failure in the current form only tells you "the test named 'rejects spaces' failed" rather than which input/expected pair broke — and it takes more prose to add a new case. The same applies to the three new `it` blocks in `deriveBranchName.test.ts` ("uses a custom prefix", "supports an empty prefix", "applies a custom prefix to the task-ID fallback"), and to the two-assertion `"supports a custom prefix"` test for `suggestBranchName` which packs two independent scenarios into one case.
Reviews (1): Last reviewed commit: "feat(settings): configurable branch name..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
114
to
150
| ).toBe("posthog-code/fix-bug-4"); | ||
| }); | ||
|
|
||
| it("supports a custom prefix", () => { | ||
| expect(suggestBranchName("Fix bug", "abc", [], "team/")).toBe( | ||
| "team/fix-bug", | ||
| ); | ||
| expect(suggestBranchName("Fix bug", "abc", ["team/fix-bug"], "team/")).toBe( | ||
| "team/fix-bug-2", | ||
| ); | ||
| }); | ||
| }); | ||
|
|
||
| describe("validateBranchPrefix", () => { | ||
| it("allows an empty prefix (no prefix)", () => { | ||
| expect(validateBranchPrefix("")).toBeNull(); | ||
| }); | ||
|
|
||
| it("allows normal prefixes", () => { | ||
| expect(validateBranchPrefix("team/")).toBeNull(); | ||
| expect(validateBranchPrefix("posthog-code/")).toBeNull(); | ||
| }); | ||
|
|
||
| it("rejects a leading dash (flag-like)", () => { | ||
| expect(validateBranchPrefix("-x/")).toBe( | ||
| "Branch prefix cannot start with a dash.", | ||
| ); | ||
| }); | ||
|
|
||
| it("rejects spaces", () => { | ||
| expect(validateBranchPrefix("my team/")).not.toBeNull(); | ||
| }); | ||
|
|
||
| it('rejects ".."', () => { | ||
| expect(validateBranchPrefix("../")).not.toBeNull(); | ||
| }); | ||
| }); |
Contributor
There was a problem hiding this comment.
validateBranchPrefix
The five separate it blocks under validateBranchPrefix all call the same function with different inputs; this is exactly the pattern the team prefers to express as it.each. A failure in the current form only tells you "the test named 'rejects spaces' failed" rather than which input/expected pair broke — and it takes more prose to add a new case. The same applies to the three new it blocks in deriveBranchName.test.ts ("uses a custom prefix", "supports an empty prefix", "applies a custom prefix to the task-ID fallback"), and to the two-assertion "supports a custom prefix" test for suggestBranchName which packs two independent scenarios into one case.
Context Used: Do not attempt to comment on incorrect alphabetica... (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/core/src/git-interaction/branchName.test.ts
Line: 114-150
Comment:
**Prefer parameterised tests for `validateBranchPrefix`**
The five separate `it` blocks under `validateBranchPrefix` all call the same function with different inputs; this is exactly the pattern the team prefers to express as `it.each`. A failure in the current form only tells you "the test named 'rejects spaces' failed" rather than which input/expected pair broke — and it takes more prose to add a new case. The same applies to the three new `it` blocks in `deriveBranchName.test.ts` ("uses a custom prefix", "supports an empty prefix", "applies a custom prefix to the task-ID fallback"), and to the two-assertion `"supports a custom prefix"` test for `suggestBranchName` which packs two independent scenarios into one case.
**Context Used:** Do not attempt to comment on incorrect alphabetica... ([source](https://app.greptile.com/review/custom-context?memory=instruction-0))
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Addresses review feedback: use it.each for validateBranchPrefix, the deriveBranchName custom-prefix cases, and suggestBranchName, matching the parameterised-test convention in AGENTS.md. Generated-By: PostHog Code Task-Id: 8e9f327d-84b1-4608-9f48-c3038dbf87ca
Author
|
Thanks for the review! Parameterised those tests with |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The branch name PostHog Code generates for a task is hardcoded with the
posthog-code/prefix. Some users want a different prefix — or none — to match their team's branch conventions.Closes #1632
Changes
branchPrefix), defaulting to the currentposthog-code/so existing behavior is unchanged.deriveBranchName/suggestBranchNametake an optionalprefixargument (defaultBRANCH_PREFIX); existing callers are unaffected.getSuggestedBranchNamereads the setting, so the generated branch name honors it wherever the app suggests one (Create PR, "Branch here").validateBranchPrefix): rejects names git would reject and a leading-; empty = no prefix. Invalid input shows an inline error and isn't persisted.How did you test this?
pnpm --filter @posthog/core --filter @posthog/ui typecheck— clean.biome lint packages/core/src/git-interaction+ Biome check on all changed files — clean, nonoRestrictedImports.vitest runonbranchName.test.ts+deriveBranchName.test.ts— 38 passing (added: custom prefix, empty prefix, task-ID fallback with prefix,validateBranchPrefix).Automatic notifications
Created with PostHog Code