feat: add enabled public API#137
Conversation
|
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
test/posthog_test.exs:24-33
**Prefer parameterised tests**
The two tests inside `enabled?/1` exercise the same function with different inputs (enabled with a key, disabled without), making them a natural fit for parameterisation. The current split into separate tests with different `@tag config:` annotations duplicates the structural boilerplate. A single table-driven loop over `{config, supervisor_name, expected}` tuples would express the same idea more concisely and satisfy the project's "OnceAndOnlyOnce" guideline.
Reviews (1): Last reviewed commit: "feat: add enabled public API" | Re-trigger Greptile |
| describe "enabled?/1" do | ||
| test "returns true when the PostHog instance has an API key" do | ||
| assert PostHog.enabled?() | ||
| end | ||
|
|
||
| @tag config: [api_key: "", supervisor_name: DisabledPostHog] | ||
| test "returns false when the PostHog instance has no API key" do | ||
| refute PostHog.enabled?(DisabledPostHog) | ||
| end | ||
| end |
There was a problem hiding this comment.
The two tests inside enabled?/1 exercise the same function with different inputs (enabled with a key, disabled without), making them a natural fit for parameterisation. The current split into separate tests with different @tag config: annotations duplicates the structural boilerplate. A single table-driven loop over {config, supervisor_name, expected} tuples would express the same idea more concisely and satisfy the project's "OnceAndOnlyOnce" guideline.
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: test/posthog_test.exs
Line: 24-33
Comment:
**Prefer parameterised tests**
The two tests inside `enabled?/1` exercise the same function with different inputs (enabled with a key, disabled without), making them a natural fit for parameterisation. The current split into separate tests with different `@tag config:` annotations duplicates the structural boilerplate. A single table-driven loop over `{config, supervisor_name, expected}` tuples would express the same idea more concisely and satisfy the project's "OnceAndOnlyOnce" guideline.
**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!
💡 Motivation and Context
Adds a small public API,
PostHog.enabled?/1, so we can test the public API snapshot workflow added in #136.Note:
public_api.snapshotis intentionally not committed in this PR.💚 How did you test it?
mix formatmix test test/posthog_test.exsmix testmix posthog.public_api --check(expected failure because the snapshot was intentionally not updated)📝 Checklist
If releasing new changes
sampo addto generate a changeset file