Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Make H2ProtocolConfig interface backward compatible #2361

Merged
merged 2 commits into from
Sep 12, 2022
Merged

Make H2ProtocolConfig interface backward compatible #2361

merged 2 commits into from
Sep 12, 2022

Conversation

idelpivnitskiy
Copy link
Member

Motivation:

#2341 added 3 new methods on H2ProtocolConfig interface but didn't provide default implementations.

Modifications:

  • Add default impls for initialSettings(), flowControlQuantum(), and flowControlWindowIncrement();

Result:

H2ProtocolConfig is backward compatible with 0.42.16 release.

@idelpivnitskiy idelpivnitskiy self-assigned this Sep 12, 2022
@idelpivnitskiy
Make H2ProtocolConfig interface backward compatible
f9d399e 
Motivation:

apple#2341 added 3 new methods on `H2ProtocolConfig` interface but didn't
provide default implementations.

Modifications:

- Add default impls for `initialSettings()`, `flowControlQuantum()`,
and `flowControlWindowIncrement()`;

Result:

`H2ProtocolConfig` is backward compatible with 0.42.16 release.
@idelpivnitskiy
Copy link
Member Author

It's weird, but this is what I got from japicmp.sh:

Comparing binary compatibility of servicetalk-http-netty-0.42.17-SNAPSHOT.jar against servicetalk-http-netty-0.42.16.jar
No changes.

Tried ./scripts/japicmp.sh 0.42.16 0.42.17-SNAPSHOT (pulls snapshot from maven central) and ./scripts/japicmp.sh 0.42.16 local after executing ./gradlew assemble --rerun-tasks, result is the same. Can we trust japicmp.sh?

@tkountis
Copy link
Contributor

It's weird, but this is what I got from japicmp.sh:

Comparing binary compatibility of servicetalk-http-netty-0.42.17-SNAPSHOT.jar against servicetalk-http-netty-0.42.16.jar
No changes.

Tried ./scripts/japicmp.sh 0.42.16 0.42.17-SNAPSHOT (pulls snapshot from maven central) and ./scripts/japicmp.sh 0.42.16 local after executing ./gradlew assemble --rerun-tasks, result is the same. Can we trust japicmp.sh?

I don't recall seeing that with the "local" option.
Do you mean that it fetches 0.42.17-SNAPSHOT from maven even when you say local?

@idelpivnitskiy
Copy link
Member Author

No, with local you have to assemble modules locally

@idelpivnitskiy
Copy link
Member Author

Tried with japicmp 0.16.0 (#2362), still no luck.

@idelpivnitskiy idelpivnitskiy merged commit 93ff14e into apple:main Sep 12, 2022
@idelpivnitskiy idelpivnitskiy deleted the H2ProtocolConfig branch September 12, 2022 21:52
@idelpivnitskiy idelpivnitskiy mentioned this pull request Sep 19, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tkountis tkountis tkountis approved these changes

@Scottmitch Scottmitch Awaiting requested review from Scottmitch

Assignees

@idelpivnitskiy idelpivnitskiy

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@idelpivnitskiy @tkountis