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

[Task] Remove disconnected nodes functionality (feature | tests | docs) #86

Closed
zinoadidi opened this issue Mar 28, 2021 · 5 comments · Fixed by #97
Closed

[Task] Remove disconnected nodes functionality (feature | tests | docs) #86

zinoadidi opened this issue Mar 28, 2021 · 5 comments · Fixed by #97
Assignees
Labels
Paid PR Reward - 300 Reward in coin(s)

Comments

@zinoadidi
Copy link
Contributor

Task

Contributors are able to complete this task and earn thenewboston coins. Check out the labels to learn how much you can earn for contributing by completing this task. Please make sure to be honest if you wish to contribute by saying you can't finish this and we can just un-assign you with no harm done! There is no point in delaying tasks from being completed for miscommunication!

Overview

Name of function Class To add Function Location in Account Manager Function Description Comments
clean Confirmation Validator, Bank src/renderer/hooks/useCleanSockets.tsx Remove disconnected nodes from the network

Behavior

/* code examples are awesome */

Please ask for this task to be assigned to you and earn and its sweet reward 😉

Remember to include your account number in your PR description for us to pay you 💰

@zinoadidi zinoadidi added the PR Reward - 300 Reward in coin(s) label Mar 28, 2021
@tomijaga
Copy link
Contributor

I'll take this too. This has the same lvl of difficulty as the crawl function

@zinoadidi
Copy link
Contributor Author

zinoadidi commented Mar 29, 2021

@tomijaga can you help review #89
I had the impression docs are autogenerated using JS Docs;
If not then you owe me some more docs based on PR
also is this the implementation we are expecting for this situation, I was of the opinion we want to get the info a better way?

@tomijaga
Copy link
Contributor

tomijaga commented Mar 29, 2021

@zinoadidi I dont think the docs are auto generated.
What PR are you referring to?
Are we meant to document all the functions or just the ones that will be directly accessed by users?

@tomijaga
Copy link
Contributor

@zinoadidi

also is this the implementation we are expecting for this situation, I was of the opinion we want to get the info a better way?

I was thinking that getBankPV() would return the PrimaryValidator with the url set to the url of the bank's pv.

@zinoadidi
Copy link
Contributor Author

OK, now i know to be on the lookout for public docs as well;
would be nice if we could autogenerate them somehow from the comments in code as opposed to doing it twice for public methods

I do agree that we do not need in the public doc information about methods and classes which are not used for public interfaces. comment on top of method is sufficient

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Paid PR Reward - 300 Reward in coin(s)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants