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

More grpc endpoints #76

Merged
merged 18 commits into from
Nov 3, 2023
Merged

More grpc endpoints #76

merged 18 commits into from
Nov 3, 2023

Conversation

rasmus-kirk
Copy link
Contributor

@rasmus-kirk rasmus-kirk commented Oct 18, 2023 •
edited
Loading

Purpose

Implements

  • GetBlocks
  • GetFinalizedBlocks
  • GetBranches
  • GetAncestors
  • GetBlockPendingUpdates

See #70

Checklist

  • My code follows the style of this project.
  • The code compiles without warnings.
  • I have performed a self-review of the changes.
  • I have documented my code, in particular the intent of the
    hard-to-understand areas.
  • (If necessary) I have updated the CHANGELOG.

@rasmus-kirk rasmus-kirk marked this pull request as ready for review October 18, 2023 08:17
@rasmus-kirk rasmus-kirk requested review from limemloh and a user October 18, 2023 08:22
limemloh
limemloh reviewed Oct 18, 2023
View reviewed changes
src/Types/ArrivedBlockInfo.cs Outdated Show resolved Hide resolved
src/Types/FinalizedBlockInfo.cs Outdated Show resolved Hide resolved
src/Types/PendingUpdate.cs Outdated Show resolved Hide resolved
src/Types/PendingUpdate.cs Outdated Show resolved Hide resolved
ghost
ghost suggested changes Oct 18, 2023
View reviewed changes
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Looks really good @rasmus-kirk 🚀

None of the examples are added to the solution project. Because of this, if you open the solution in standard IDE’s like Rider or Visual Studio the examples are not shown.

Please, as the other examples, add the example projects to the solution project :-)

CHANGELOG.md Outdated Show resolved Hide resolved
examples/GetAncestors/Program.cs Outdated Show resolved Hide resolved
examples/GetBranches/Program.cs Outdated Show resolved Hide resolved
examples/GetBlocks/Program.cs Show resolved Hide resolved
examples/GetBranches/Program.cs Outdated Show resolved Hide resolved
examples/GetFinalizedBlocks/Program.cs Outdated Show resolved Hide resolved
src/Client/ConcordiumClient.cs Show resolved Hide resolved
src/Types/PendingUpdate.cs Outdated Show resolved Hide resolved
src/Types/PendingUpdate.cs Outdated Show resolved Hide resolved
limemloh
limemloh requested changes Oct 18, 2023
View reviewed changes
Copy link
Collaborator

@limemloh limemloh left a comment

Choose a reason for hiding this comment

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

Well done, only relatively minor comments from my side.

src/Types/PendingUpdate.cs Outdated Show resolved Hide resolved
src/Client/ConcordiumClient.cs Outdated Show resolved Hide resolved
examples/GetBlockPendingUpdates/Program.cs Outdated Show resolved Hide resolved
examples/GetBlockPendingUpdates/Program.cs Outdated Show resolved Hide resolved
examples/GetAncestors/Program.cs Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
rasmus-kirk and others added 2 commits October 26, 2023 06:54
@rasmus-kirk @limemloh
Apply suggestions from code review
03e50ea 
Co-authored-by: Emil Holm Gjørup <eg@concordium.com>
@rasmus-kirk
Adressed comments
fc138b7
@rasmus-kirk rasmus-kirk requested review from limemloh and a user October 26, 2023 08:13
ghost
ghost approved these changes Oct 26, 2023
View reviewed changes
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

One comment - nice work :-)

CHANGELOG.md Outdated Show resolved Hide resolved
limemloh
limemloh approved these changes Oct 26, 2023
View reviewed changes
Copy link
Collaborator

@limemloh limemloh left a comment

Choose a reason for hiding this comment

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

Looks good to me, assuming you fix my minor comments

src/Types/TransactionTime.cs Outdated Show resolved Hide resolved
src/Client/ConcordiumClient.cs Outdated Show resolved Hide resolved
src/Client/ConcordiumClient.cs Show resolved Hide resolved
@rasmus-kirk rasmus-kirk merged commit cdadc98 into main Nov 3, 2023
1 check passed
@rasmus-kirk rasmus-kirk deleted the more-grpc-endpoints branch November 3, 2023 08:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@abizjak abizjak abizjak left review comments

@limemloh limemloh limemloh approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rasmus-kirk @abizjak @limemloh