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

Implement/implement client #57

Draft
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

18aaddy
Copy link

@18aaddy 18aaddy commented Sep 21, 2024

Resolves #38

  • Created rpc.go, node.go and errors.go
  • Used go routines and channels instead of context library

The cli folder and client.go are also there because I had started working on them beforehand

@18aaddy
Copy link
Author

18aaddy commented Sep 21, 2024

@star-gazer111 pls review

@star-gazer111
Copy link
Member

star-gazer111 commented Sep 21, 2024

@star-gazer111 pls review

So u have implemented the entire client module as well as the entire CLI module in this PR?

@18aaddy
Copy link
Author

18aaddy commented Sep 22, 2024

@star-gazer111 pls review

So u have implemented the entire client module as well as the entire CLI module in this PR?

No I have implemented only node.go, rpc.go, and errors.go. I had started some part of cli module and client.go

@star-gazer111
Copy link
Member

@star-gazer111 pls review

So u have implemented the entire client module as well as the entire CLI module in this PR?

No I have implemented only node.go, rpc.go, and errors.go. I had started some part of cli module and client.go

Can you please create separate PRs out of this?

  • one for client.go
  • one for cli.go
  • one for node,rpc & errors

@VeerChaurasia
Copy link

@star-gazer111 @18aaddy picking up cli.go from here

@star-gazer111
Copy link
Member

@star-gazer111 @18aaddy picking up cli.go from here

@18aaddy u can create 2 PRs now from here one involving client.go & the other involving node,rpc & errors. You can remove the commits for CLI module.

@18aaddy 18aaddy mentioned this pull request Sep 23, 2024
@star-gazer111
Copy link
Member

@18aaddy can you rebase your branch please and then make the commits. Else it's leading to a merge conflict.

@18aaddy
Copy link
Author

18aaddy commented Sep 24, 2024

@18aaddy can you rebase your branch please and then make the commits. Else it's leading to a merge conflict.

Done
Also in this commit I have updated two functions of config/config.go that were not exported earlier

@gerceboss
Copy link
Contributor

@18aaddy , I dont think this PR has been split yet

@18aaddy
Copy link
Author

18aaddy commented Oct 27, 2024

@gerceboss Should I split it now or wait till rest of files in execution are merged because I'll need to update this PR then anyway as Client depends on execution

@star-gazer111
Copy link
Member

@gerceboss Should I split it now or wait till rest of files in execution are merged because I'll need to update this PR then anyway as Client depends on execution

wait for other PRs first

@18aaddy 18aaddy marked this pull request as draft November 4, 2024 12:34
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.

4 participants