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

feat: add path type with metadata and basic daemon requests #24

Merged
merged 10 commits into from
Nov 21, 2023
Merged

feat: add path type with metadata and basic daemon requests #24

merged 10 commits into from
Nov 21, 2023

Conversation

mlegner
Copy link
Contributor

@mlegner mlegner commented Oct 6, 2023 •
edited
Loading

Closes #12

@mlegner mlegner added enhancement New feature or request rust labels Oct 6, 2023
@mlegner mlegner self-assigned this Oct 6, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Oct 6, 2023 •
edited
Loading

Code Coverage

Package Line Rate Health
crates/scion/src/path 77% ✔
crates/scion/src/path/metadata 100% ✔
crates/scion/src/daemon 64% ➖
crates/scion/src/address 71% ➖
crates/scion/src/reliable 94% ✔
crates/scion/src 78% ✔
crates/scion/src/packet 65% ➖
Summary 78% (662 / 851) ✔

@mlegner mlegner force-pushed the feat/daemon-conversion branch 3 times, most recently from d368d31 to d2eca12 Compare October 9, 2023 16:15
@mlegner mlegner force-pushed the feat/daemon-conversion branch 6 times, most recently from c46c8af to 0475c38 Compare November 15, 2023 15:47
jpcsmith
jpcsmith reviewed Nov 16, 2023
View reviewed changes
Copy link
Contributor

@jpcsmith jpcsmith left a comment

Choose a reason for hiding this comment

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

Hey Markus, it's looking good. I added some suggestions for consideration.

crates/scion/src/daemon/messages.rs Outdated Show resolved Hide resolved
crates/scion/src/daemon/messages.rs Outdated Show resolved Hide resolved
crates/scion/src/path/cp_path.rs Outdated Show resolved Hide resolved
crates/scion/src/path/cp_path.rs Outdated Show resolved Hide resolved
crates/scion/src/path/cp_path.rs Outdated Show resolved Hide resolved
crates/scion/src/path/cp_path.rs Outdated Show resolved Hide resolved
crates/scion/src/path.rs Outdated Show resolved Hide resolved
crates/scion/src/daemon/client.rs Outdated Show resolved Hide resolved
crates/scion/src/daemon/client.rs Outdated Show resolved Hide resolved
crates/scion/src/daemon/client.rs Outdated Show resolved Hide resolved
@mlegner
Copy link
Contributor Author

mlegner commented Nov 17, 2023

Thanks a lot for the great suggestions, @jpcsmith! I'll update the PR and ping you again once it's ready.

@mlegner mlegner marked this pull request as ready for review November 20, 2023 15:02
@mlegner mlegner mentioned this pull request Nov 20, 2023
Closed
2 tasks
jpcsmith
jpcsmith requested changes Nov 20, 2023
View reviewed changes
Copy link
Contributor

@jpcsmith jpcsmith left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Hey Markus, it looks good. The only remaining issue is that there seems to be a duplicated struct. Once that's resolved, it's good to merge.

Feel free to simply ignore and resolve the stylistic changes.

crates/scion/src/daemon/client.rs Outdated Show resolved Hide resolved
crates/scion/src/path/metadata.rs Outdated Show resolved Hide resolved
crates/scion/src/path/metadata.rs Outdated Show resolved Hide resolved
crates/scion/src/path/metadata.rs Outdated Show resolved Hide resolved
crates/scion/src/path/metadata.rs Show resolved Hide resolved
jpcsmith
jpcsmith approved these changes Nov 21, 2023
View reviewed changes
Copy link
Contributor

@jpcsmith jpcsmith left a comment

Choose a reason for hiding this comment

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

Hey Markus, thanks for implementing all this, nice work.

crates/scion/src/daemon/client.rs Outdated Show resolved Hide resolved
@mlegner mlegner changed the title feat(daemon): add basic requests feat: add path type with metadata and basic daemon requests Nov 21, 2023
@mlegner mlegner merged commit f2dc8c2 into main Nov 21, 2023
11 checks passed
@mlegner mlegner deleted the feat/daemon-conversion branch November 21, 2023 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jpcsmith jpcsmith jpcsmith approved these changes

Assignees

@mlegner mlegner

Labels
enhancement New feature or request rust
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Implement interactions with SCION daemon
2 participants
@mlegner @jpcsmith