Skip to content

Change API for EAD items to support large numbers#333

Merged
chrysn merged 2 commits intolake-rs:mainfrom
chrysn-pull-requests:ead-api-16
Feb 3, 2025
Merged

Change API for EAD items to support large numbers#333
chrysn merged 2 commits intolake-rs:mainfrom
chrysn-pull-requests:ead-api-16

Conversation

@chrysn
Copy link
Copy Markdown
Member

@chrysn chrysn commented Jan 22, 2025

The set of supported values is not changed, but the API is changed so that the implementation can be fixed without an extra breaking change.


Didn't run any special tests, let's see if CI is happy. (Should be a no-brainer though).

@chrysn
Copy link
Copy Markdown
Member Author

chrysn commented Jan 29, 2025

I had a hunch I'd done this before already: this was my first PR (https://github.com/openwsn-berkeley/lakers/pull/100/files). But too much has changed, so the old PR is not actionable any more.

It did more than just change the type (it made its details private); that's still something worth doing, but maybe better deferred to be done together with other restructurings like supporting multiple EAD items.

@chrysn
Copy link
Copy Markdown
Member Author

chrysn commented Feb 3, 2025

Rebased onto main to fix CI; I think this is ready for review.

@chrysn
Copy link
Copy Markdown
Member Author

chrysn commented Feb 3, 2025

Latest fix also adjusts for the language bindings that did not get tested as part of cargo test; relying on CI to check whether those are still good.

@geonnave
Copy link
Copy Markdown
Collaborator

geonnave commented Feb 3, 2025

Almost ready to merge, can you add ead: as prefix to the commit messages?

The set of supported values is not changed, but the API is changed so
that the implementation can be fixed without an extra breaking change.
@chrysn
Copy link
Copy Markdown
Member Author

chrysn commented Feb 3, 2025

Fixed!

@chrysn chrysn merged commit 747368a into lake-rs:main Feb 3, 2025
@chrysn chrysn deleted the ead-api-16 branch February 3, 2025 22:19
@geonnave geonnave mentioned this pull request Mar 16, 2025
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