feat: add tagged to Journaled::Change - #82
Conversation
Add support for the gem's existing "tagged" mechanism to `Journaled::Change`, so apps that use `Journaled.tag!`/`Journaled.tagged` to attach per-request context (e.g. actor metadata, request ID, span, job ID, etc) can now see that context on change events.
|
|
||
| module Journaled | ||
| VERSION = "6.2.9" | ||
| VERSION = "6.3.0" |
There was a problem hiding this comment.
This is a breaking change for anyone already consuming the event, so we'll want to go to 7.0.0 here.
There was a problem hiding this comment.
(specifically, the change to journaled_schemas/journaled/change.json means that consumers will need to handle tags even if it's empty / not present by default)
| it 'includes :tags only in the tagged instance\'s payload' do | ||
| tagged_change | ||
|
|
||
| expect(untagged_change.journaled_attributes.keys).not_to include(:tags) |
There was a problem hiding this comment.
👍 this is the desired behavior that gives consumers a migration path.
smudge
left a comment
There was a problem hiding this comment.
happy to debate whether or not this is truly merits a major version bump, but looking at this from the perspective of consumers, the gem now allows for a local implementation change that will pass CI checks but then potentially break consumers in production, so I was thinking that we could use the major release to flag that.
Alternatively, we would need some way of detecting and failing if someone adds tagged: true to an existing directive (forcing them to acknowledge that consumers are prepared for this new field), but I wasn't seeing a clear path to incorporating such a check into the consuming app's own runtime behavior (such that the consuming app would have a chance to have its own tests fail).
Add support for the gem's existing "tagged" mechanism to
Journaled::Change, so apps that useJournaled.tag!/Journaled.taggedto attach per-request context (e.g. actor metadata, request ID, span, job ID, etc) can now see that context on change events.