Skip to content

Add timestamp to Attachment #305

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jul 7, 2025
Merged

Add timestamp to Attachment #305

merged 4 commits into from
Jul 7, 2025

Conversation

davidjgoss
Copy link
Contributor

@davidjgoss davidjgoss commented Jul 2, 2025

🤔 What's changed?

Add Attachment.timestamp

⚡️ What's your motivation?

Fixes #304.

During test and step executions the execution might produce multiple attachments. For longer steps the exact timing of when the attachments were captured might be important. Currently the Attachment envelop[e] type does not have a timestamp and therefore this information cannot be captured and used.

It makes sense to do this now because we already have the signature change for Attachment in #301 which isn't released yet.

🏷️ What kind of change is this?

  • ⚡ New feature (non-breaking change which adds new behaviour)

📋 Checklist:

  • I agree to respect and uphold the Cucumber Community Code of Conduct
  • I've changed the behaviour of the code
    • I have added/updated tests to cover my changes.
  • My change requires a change to the documentation.
    • I have updated the documentation accordingly.
  • Users should know about my change
    • I have added an entry to the "Unreleased" section of the CHANGELOG, linking to this pull request.

This text was originally generated from a template, then edited by hand. You can modify the template here.

@davidjgoss davidjgoss marked this pull request as ready for review July 2, 2025 18:09
Copy link
Contributor

@ehuelsmann ehuelsmann left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@luke-hill
Copy link
Contributor

Question (More from the other PR), should we add to the changelog that the prior attribute is now deprecated and will be removed in the next 1/2/3 majors time?

Copy link
Contributor

@mpkorstanje mpkorstanje left a comment

Choose a reason for hiding this comment

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

LGTM, but this is a breaking change.

@davidjgoss
Copy link
Contributor Author

Question (More from the other PR), should we add to the changelog that the prior attribute is now deprecated and will be removed in the next 1/2/3 majors time?

Good point, will do that in a separate commit before releasing.

@davidjgoss
Copy link
Contributor Author

LGTM, but this is a breaking change.

Agreed, will release as a major.

@mpkorstanje mpkorstanje merged commit 843ce7b into main Jul 7, 2025
38 checks passed
@mpkorstanje mpkorstanje deleted the attachment-timestamp branch July 7, 2025 11:08
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.

Add Attachment.Timestamp to the schema
4 participants