Skip to content

Conversation

@ok-nick
Copy link
Contributor

@ok-nick ok-nick commented Jun 5, 2025

Changes in this pull request

Implements the Soft Binding assertion definition. It exposes all public fields so it's clear in the docs how to deserialize into it and it provides no functions since everything is already accessible by field. Getters/setters can be added on if wanted, but there is already a lot of variation in the implementation of assertions.

Checklist

  • This PR represents a single feature, fix, or change.
  • All applicable changes have been documented.
  • Any TO DO items (or similar) have been entered as GitHub issues and the link to that issue has been included in a comment.

@ok-nick ok-nick force-pushed the ok-nick/soft-binding branch from cca0a83 to d189d86 Compare June 5, 2025 16:46
Copy link
Collaborator

@gpeacock gpeacock left a comment

Choose a reason for hiding this comment

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

We need unit tests for this as well as some level of integration test to show how it would be used in practice.

@ok-nick ok-nick requested a review from gpeacock June 18, 2025 13:57
@ok-nick ok-nick requested a review from gpeacock July 16, 2025 18:36
Copy link
Collaborator

@gpeacock gpeacock left a comment

Choose a reason for hiding this comment

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

I think this is ready to go but I'd like to not add the practice of appending Map to field names unless our plan is to do that everywhere, and I'm not sure what it adds.

@ok-nick ok-nick requested a review from gpeacock August 11, 2025 15:06
@ok-nick ok-nick merged commit c3e68aa into main Aug 11, 2025
21 checks passed
@ok-nick ok-nick deleted the ok-nick/soft-binding branch August 11, 2025 21:25
@caiopensrc caiopensrc mentioned this pull request Aug 11, 2025
ok-nick added a commit that referenced this pull request Aug 13, 2025
* feat: impl Soft Binding assertion definition

* fix: remove `AssertionJson` impl for `SoftBinding`

* fix: use usize for timespan

* docs: clarify `alg_params` field string type

* style: move soft binding pad fields to the bottom of struct

* fix: soft binding region field optional and add unit + integration tests

* fix: clippy lints

* fix: use u64 instead of usize for soft binding time range

* fix: hide and skip serializing deprecated soft binding fields and expose accessor

* test: ignore deprecated url field in soft binding serialization test

* docs: document deprecated methods and remove assertion creation version

* refactor: move SoftBinding::pad/pad2 fields as methods and remove Map suffix from related structs

* fix: `SoftBinding::alg_params`->`SoftBinding::alg-params` w/ serde

---------

Co-authored-by: Eric Scouten <scouten@adobe.com>
Co-authored-by: Gavin  Peacock <gpeacock@adobe.com>
@caiopensrc caiopensrc mentioned this pull request Aug 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants