Skip to content

Tidy the public API before 1.0 - #52

Open
loevgaard wants to merge 1 commit into
fix/29-cookie-domainfrom
fix/27-public-api
Open

Tidy the public API before 1.0#52
loevgaard wants to merge 1 commit into
fix/29-cookie-domainfrom
fix/27-public-api

Conversation

@loevgaard

Copy link
Copy Markdown
Member

Fixes #27

Stacked on #51.

Problem

The Roave BC check will lock the public surface in at 1.0, and a few things were public but unused, undocumented, or both.

Change

Kept and documented

  • ConversionsApiEventRaised::$context is the way to hand your own listeners data that must never reach Meta, such as the order the event was raised for. It now says so in the docblock and in README.md, and it gained the getContext(string $key, mixed $default = null) accessor it was missing next to hasContext().
  • CommandInterface is genuinely useful for routing every command the bundle dispatches as a group. The docblock now shows the routing snippet, and the README mentions it.

Tightened

  • ConversionsApiEventRaised::$event and ::$context are readonly. The Event object stays mutable, which is what enrichment listeners need, but the properties can no longer be swapped.

Removed

  • The setono_meta_conversions_api.client_side.enabled and .server_side.enabled container parameters. No service read them once the subscribers became conditional, and whether a side is on is already visible from whether its services exist.

Tests

A test for getContext() including the default, and the extension test drops its assertions on the two removed parameters.

@codecov

codecov Bot commented Sep 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (fix/29-cookie-domain@2844117). Learn more about missing BASE report.

Additional details and impacted files
@@                   Coverage Diff                   @@
##             fix/29-cookie-domain      #52   +/-   ##
=======================================================
  Coverage                        ?   82.29%           
  Complexity                      ?      165           
=======================================================
  Files                           ?       34           
  Lines                           ?      531           
  Branches                        ?        0           
=======================================================
  Hits                            ?      437           
  Misses                          ?       94           
  Partials                        ?        0           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

The Roave BC check will lock this surface in, so decide on the parts
that were public but unused.

Keep the event context and document it as the way to pass data to your
own listeners, adding a getContext() accessor; keep CommandInterface and
document routing by interface; make the event properties readonly; and
drop the two container parameters no service reads.

Fixes #27
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