Skip to content

feat: add enabled public API#137

Closed
marandaneto wants to merge 1 commit into
PostHog:mainfrom
marandaneto:feat/add-enabled-public-api
Closed

feat: add enabled public API#137
marandaneto wants to merge 1 commit into
PostHog:mainfrom
marandaneto:feat/add-enabled-public-api

Conversation

@marandaneto

Copy link
Copy Markdown
Member

💡 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.snapshot is intentionally not committed in this PR.

💚 How did you test it?

  • mix format
  • mix test test/posthog_test.exs
  • mix test
  • mix posthog.public_api --check (expected failure because the snapshot was intentionally not updated)

📝 Checklist

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • I updated the docs if needed.
  • No breaking change or entry added to the changelog.

If releasing new changes

  • Ran sampo add to generate a changeset file

@marandaneto

Copy link
Copy Markdown
Member Author

** (Mix) Public API snapshot is out of date.

Run mix posthog.public_api --update and commit public_api.snapshot.
To inspect the change locally, run mix posthog.public_api --update and then git diff -- public_api.snapshot.

Diff:
diff --git a/public_api.snapshot b/tmp/public_api.snapshot.generated-2596
index b20b4c7..40caa89 100644
--- a/public_api.snapshot
+++ b/tmp/public_api.snapshot.generated-2596
@@ -12,6 +12,8 @@ PostHog
capture/3 doc: documented
config/0 doc: none
config/1 doc: documented

  • enabled?/0 doc: none
  • enabled?/1 doc: documented
    get_context/0 doc: none
    get_context/1 doc: documented
    get_event_context/1 doc: none

Error: Process completed with exit code 1.

@greptile-apps

greptile-apps Bot commented Jun 15, 2026

Copy link
Copy Markdown
Prompt To Fix All With AI
Fix 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

Comment thread test/posthog_test.exs
Comment on lines +24 to +33
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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 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)

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant