refactor: merge neqo-client and neqo-server into neqo-bin#1724
Merged
Conversation
mxinden
requested review from
KershawChang,
larseggert and
martinthomson
as code owners
This commit merges the `neqo-client` and `neqo-server` crates into a single new `neqo-bin` crate. In addition it de-duplicates command line argument definitions by introducing a `SharedArgs` `struct`. Future commits will consolidate more shared code into `neqo-bin/src/lib.rs` and its sub-modules. Note that binary names are unchanged. E.g. one can still run the client via `cargo run --bin neqo-client`. See `neqo-bin/Cargo.toml` for binary declarations.
mxinden
force-pushed
the
merge-client-server-2
branch
from
1b23ae1 to
d897122
Compare
larseggert
approved these changes
Mar 8, 2024
larseggert
left a comment
larseggert
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, but would like to hear from the others.
martinthomson
approved these changes
Mar 10, 2024
mxinden
added a commit
to mxinden/neqo
that referenced
this pull request
Mar 16, 2024
Before mozilla#1724 `neqo-server` would always run with `ConnectionParameters::pacing` `true`. `neqo-client` would provide a command line flag `--pacing` which defaulted to `false`. mozilla#1724 consolidated the `QuicParameters` `struct` from `neqo-client` and `neqo-server`. Since both `neqo-client` and `neqo-server` default to `pacing` `false`. This commit restores the pre-mozilla#1724 behavior, i.e. `neqo-server` defaulting to `pacing=true` and `neqo-client` defaulting to `pacing=false`. (Note that you will have to provide a value to `--pacing` from now on. I.e. `--pacing=true` or `--pacing=false` instead of just `--pacing`.)
larseggert
added a commit
to larseggert/neqo
that referenced
this pull request
Mar 16, 2024
Should have been part of mozilla#1724
mxinden
added a commit
to mxinden/neqo
that referenced
this pull request
Mar 17, 2024
Before mozilla#1724 `neqo-server` would always run with `ConnectionParameters::pacing` `true`. `neqo-client` would provide a command line flag `--pacing` which defaulted to `false`. mozilla#1724 consolidated the `QuicParameters` `struct` from `neqo-client` and `neqo-server`. Since both `neqo-client` and `neqo-server` default to `pacing` `false`. This commit replaces the `--pacing` flag with `--no-pacing`. Pacing is enabled by default on both `neqo-server` and `neqo-client`.
github-merge-queue Bot
pushed a commit
that referenced
this pull request
Mar 17, 2024
* fix(bin): enable pacing on server by default Before #1724 `neqo-server` would always run with `ConnectionParameters::pacing` `true`. `neqo-client` would provide a command line flag `--pacing` which defaulted to `false`. #1724 consolidated the `QuicParameters` `struct` from `neqo-client` and `neqo-server`. Since both `neqo-client` and `neqo-server` default to `pacing` `false`. This commit restores the pre-#1724 behavior, i.e. `neqo-server` defaulting to `pacing=true` and `neqo-client` defaulting to `pacing=false`. (Note that you will have to provide a value to `--pacing` from now on. I.e. `--pacing=true` or `--pacing=false` instead of just `--pacing`.) * fix(bin): enable pacing by default and add optional --no-pacing Before #1724 `neqo-server` would always run with `ConnectionParameters::pacing` `true`. `neqo-client` would provide a command line flag `--pacing` which defaulted to `false`. #1724 consolidated the `QuicParameters` `struct` from `neqo-client` and `neqo-server`. Since both `neqo-client` and `neqo-server` default to `pacing` `false`. This commit replaces the `--pacing` flag with `--no-pacing`. Pacing is enabled by default on both `neqo-server` and `neqo-client`. * Update test/upload_test.sh * Update comment
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This commit merges the
neqo-clientandneqo-servercrates into a single newneqo-bincrate. In addition it de-duplicates command line argument definitions by introducing aSharedArgsstruct.Future commits will consolidate more shared code into
neqo-bin/src/lib.rsand its sub-modules.Note that binary names are unchanged. E.g. one can still run the client via
cargo run --bin neqo-client. Seeneqo-bin/Cargo.tomlfor binary declarations.Part of #1696. See also past issue #1403.
Small step forward already:
