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.

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

Some improvements to the hydroflow template #14

Merged
merged 4 commits into from
Mar 27, 2024

Conversation

rohitkulshreshtha
Copy link
Contributor

  1. Default server address, so you don't have to copy an address around when starting multiple clients to test a full star network configuration.
  2. Help messages for all the arguments.
  3. More hand-holding documentation, mostly condensed from the first networking chapter
  4. Updated README.md to provide more runnable commands.

1. Default server address, so you don't have to copy an address around when starting multiple clients to test a full star network configuration.
2. Help messages for all the arguments.
3. More hand-holding documentation, mostly condensed from the [first networking chapter](https://hydro.run/docs/hydroflow/quickstart/example_7_echo_server)
4. Updated README.md to provide more runnable commands.
@rohitkulshreshtha rohitkulshreshtha marked this pull request as ready for review March 14, 2024 23:14
@rohitkulshreshtha
Copy link
Contributor Author

My guess is that cargo fmt --check was added recently? For example, it is failing on

Echo { payload: String, ts: DateTime<Utc> }

which has always existed...

src/main.rs Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
src/main.rs Show resolved Hide resolved
@MingweiSamuel
Copy link
Member

Oh and for cargo fmt, just run cargo fmt (or cargo +nightly fmt) and it will fix all the formatting and pass CI

Copy link
Contributor

@jhellerstein jhellerstein left a comment

Choose a reason for hiding this comment

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

LGTM. Maybe change the earlier advice to use for_each(...println!...) to say inspect(...println!...)?

The hydroflow template testing step needs to be updated to use the address simplification.
@rohitkulshreshtha rohitkulshreshtha merged commit 5d68cd7 into main Mar 27, 2024
2 checks passed
@rohitkulshreshtha rohitkulshreshtha deleted the rohit_template_improvements branch March 27, 2024 17:50
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.

None yet

3 participants