Skip to content

feat: add tagged to Journaled::Change - #82

Open
stevenjackson wants to merge 2 commits into
Betterment:masterfrom
stevenjackson:sj/add-tagged-to-change
Open

feat: add tagged to Journaled::Change#82
stevenjackson wants to merge 2 commits into
Betterment:masterfrom
stevenjackson:sj/add-tagged-to-change

Conversation

@stevenjackson

Copy link
Copy Markdown

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.

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.
@stevenjackson
stevenjackson requested a review from a team as a code owner August 14, 2026 20:05
Comment thread lib/journaled/version.rb Outdated

module Journaled
VERSION = "6.2.9"
VERSION = "6.3.0"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is a breaking change for anyone already consuming the event, so we'll want to go to 7.0.0 here.

@smudge smudge Aug 14, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Updated to 7!

it 'includes :tags only in the tagged instance\'s payload' do
tagged_change

expect(untagged_change.journaled_attributes.keys).not_to include(:tags)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍 this is the desired behavior that gives consumers a migration path.

@smudge smudge left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

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.

2 participants