Skip to content

Revert #512 and #508#663

Merged
serban300 merged 4 commits intomasterfrom
jsdw-revert-512-508
Jan 23, 2025
Merged

Revert #512 and #508#663
serban300 merged 4 commits intomasterfrom
jsdw-revert-512-508

Conversation

@jsdw
Copy link
Copy Markdown
Contributor

@jsdw jsdw commented Dec 2, 2024

Quick attempt to revert these PRs mentioned in #662 (comment)

@jsdw
Copy link
Copy Markdown
Contributor Author

jsdw commented Dec 2, 2024

For my own benefit: I realised just now that the recent releases prior to 3.7.0 all came off the 3.6.x branch, which had the above PRs reverted also. I totally missed this branch when doing the latest release, assuming everything was released off master!

I think it would be good to get master back to a sane point. Maybe we should not try to merge this PR at all (which we would do just to get 3.7.1 out, and then want to undo anyways for 4) and instead just release 4.0.0 directly now that we've yanked 3.7.0 and deal with the fallout of this major bump. Does that sound fair?

@serban300
Copy link
Copy Markdown
Contributor

For my own benefit: I realised just now that the recent releases prior to 3.7.0 all came off the 3.6.x branch, which had the above PRs reverted also. I totally missed this branch when doing the latest release, assuming everything was released off master!

I think it would be good to get master back to a sane point. Maybe we should not try to merge this PR at all (which we would do just to get 3.7.1 out, and then want to undo anyways for 4) and instead just release 4.0.0 directly now that we've yanked 3.7.0 and deal with the fallout of this major bump. Does that sound fair?

@jsdw what you're saying makes sense, but personally I would need a 3.7.1 release soon-ish in order to access the functionality introduced in #616 and I think we would have to wait quite a while for a 4.0 release

So could we have a v3.7.x branch for now that would include this PR, and then release 3.7.1 from it ? I can do it if you want. Just please let me know how you prefer

@bkchr
Copy link
Copy Markdown
Member

bkchr commented Jan 22, 2025

Let's merge this pr and add it to the 4.0 milestone (so we can take care of it in the future :P)

@serban300
Copy link
Copy Markdown
Contributor

Let's merge this pr and add it to the 4.0 milestone (so we can take care of it in the future :P)

Sounds good to me. We just need to fix the failing test (Rust CI/CD / tests (pull_request)) .@jsdw I can do it if you want. However you prefer.

@serban300 serban300 merged commit 96269d4 into master Jan 23, 2025
This was referenced Jan 23, 2025
ty.span() => .saturating_add(
<<#ty as #crate_path::HasCompact>::Type as #crate_path::MaxEncodedLen>::max_encoded_len()
)
ty.span() => .saturating_add(<#crate_path::Compact::<#ty> as #crate_path::MaxEncodedLen>::max_encoded_len())
Copy link
Copy Markdown
Contributor

@gui1117 gui1117 Jan 30, 2025

Choose a reason for hiding this comment

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

The code before was actually: ty.span() => .saturating_add(<#ty>::max_encoded_len())

This makes some issue with #691

serban300 added a commit to serban300/parity-scale-codec that referenced this pull request Jan 30, 2025
@serban300 serban300 mentioned this pull request Jan 30, 2025
serban300 added a commit to serban300/parity-scale-codec that referenced this pull request Jan 30, 2025
serban300 added a commit that referenced this pull request Jan 31, 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.

4 participants