Skip to content

Add ListKeys support #53

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

Merged
merged 3 commits into from
Oct 12, 2020
Merged

Conversation

joechrisellis
Copy link
Contributor

Rust client support for the ListKeys operation.

@joechrisellis
Copy link
Contributor Author

NOTE: won't pass CI because this is pending a version bump to parsec-interface-rs.

Copy link
Member

@hug-dev hug-dev left a comment

Choose a reason for hiding this comment

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

That looks good to me! Could you please just add a test in src/core/testing/core_tests.rs?

@joechrisellis joechrisellis force-pushed the add-list-keys branch 2 times, most recently from b0af408 to b7bcc3f Compare October 9, 2020 10:17
Copy link
Member

@hug-dev hug-dev left a comment

Choose a reason for hiding this comment

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

Thanks!
I would personally be ok to merge this, even if it contains a patch. We can't publish a crates-io version with a patch in it anyway (so no mistake is possible) and that leaves us some room to continue development until the next version without keeping this PR up.

@ionut-arm What do you think?

Cargo.toml Outdated
@@ -26,3 +26,6 @@ mockstream = "0.0.3"
[features]
testing = ["parsec-interface/testing", "no-fs-permission-check"]
no-fs-permission-check = []

[patch.crates-io]
parsec-interface = { git = "https://github.com/joechrisellis/parsec-interface-rs.git", branch = "add-list-keys" }
Copy link
Member

Choose a reason for hiding this comment

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

This should work as everything should be merged in the interface 😃

-parsec-interface = { git = "https://github.com/joechrisellis/parsec-interface-rs.git", branch = "add-list-keys" }
+parsec-interface = { git = "https://github.com/parallaxsecond/parsec-interface-rs" }

@joechrisellis joechrisellis force-pushed the add-list-keys branch 2 times, most recently from 4bfd365 to b3d3b4e Compare October 12, 2020 09:01
Cargo.toml Outdated
@@ -26,3 +26,6 @@ mockstream = "0.0.3"
[features]
testing = ["parsec-interface/testing", "no-fs-permission-check"]
no-fs-permission-check = []

[patch.crates-io]
parsec-interface = { git = "https://github.com/parallaxsecond/parsec-interface-rs.git" }
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to scope this down to a particular commit? Otherwise we could end up making breaking changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Joe Ellis added 3 commits October 12, 2020 11:44
Patched dependency to use joechrisellis/parsec-interface-rs.

Signed-off-by: Joe Ellis <[email protected]>
Signed-off-by: Joe Ellis <[email protected]>
Signed-off-by: Joe Ellis <[email protected]>
Copy link
Member

@ionut-arm ionut-arm left a comment

Choose a reason for hiding this comment

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

Thanks!!

@hug-dev hug-dev merged commit 878c45d into parallaxsecond:master Oct 12, 2020
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.

3 participants