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

Delete Bootstrap and Kademlia #2974

Merged
merged 12 commits into from
Oct 4, 2019
Merged

Delete Bootstrap and Kademlia #2974

merged 12 commits into from
Oct 4, 2019

Conversation

jenlij
Copy link
Contributor

@jenlij jenlij commented Sep 6, 2019

What: Removes the boostrap node files and usages & deletes entire kademlia package
Why: Since we are replacing kademlia with a direct node to satellite discovery process, we no longer need any bootstrap nodes

Note: Don't fret about the 84 files changed. Most are deletions or mechanical changes.

Code Review Checklist (to be filled out by reviewer)

  • Does the PR describe what changes are being made?
  • Does the PR describe why the changes are being made?
  • Does the code follow our style guide?
  • Does the code follow our testing guide?
  • Is the PR appropriately sized? (If it could be broken into smaller PRs it should be)
  • Does the new code have enough tests? (every PR should have tests or justification otherwise. Bug-fix PRs especially)
  • Does the new code have enough documentation that answers "how do I use it?" and "what does it do?"? (both source documentation and higher level, diagrams?)
  • Does any documentation need updating?
  • Do the database access patterns make sense?

@cla-bot cla-bot bot added the cla-signed label Sep 6, 2019
@jenlij jenlij marked this pull request as ready for review September 19, 2019 20:31
@jenlij jenlij requested a review from a team September 19, 2019 20:31
@ghost ghost requested review from Barterio and VinozzZ and removed request for a team September 19, 2019 20:32
@jenlij jenlij requested review from cam-a and JessicaGreben and removed request for a team, Barterio and VinozzZ September 19, 2019 20:32
@jenlij jenlij added Request Code Review Code review requested Reviewer Can Merge If all checks have passed, non-owner can merge PR labels Sep 19, 2019
@jenlij jenlij added Do Not Merge and removed Reviewer Can Merge If all checks have passed, non-owner can merge PR labels Sep 19, 2019
@jenlij jenlij mentioned this pull request Sep 19, 2019
9 tasks
@jenlij jenlij changed the title remove bootstrap Delete Bootstrap and Kademlia Sep 30, 2019
@jenlij jenlij added Reviewer Can Merge If all checks have passed, non-owner can merge PR and removed Do Not Merge labels Oct 3, 2019
// If we get a Node without an ID (i.e. bootstrap node)
// we don't want to add to the routing tbale
// If we get a Node without an ID
// we don't want to add to the cache
Copy link
Member

Choose a reason for hiding this comment

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

to the database

Copy link
Member

@egonelbre egonelbre left a comment

Choose a reason for hiding this comment

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

neat

@@ -12,6 +12,7 @@ import "google/protobuf/timestamp.proto";

package inspector;
Copy link
Member

Choose a reason for hiding this comment

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

We can change inspector packages as needed and overridde the lock file, since they are not part of the communication protocol.

Barterio
Barterio previously approved these changes Oct 4, 2019
Copy link
Contributor

@Barterio Barterio left a comment

Choose a reason for hiding this comment

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

Huge PR, But looks good to me

egonelbre
egonelbre previously approved these changes Oct 4, 2019
@jenlij jenlij merged commit 7ceaabb into master Oct 4, 2019
@jenlij jenlij deleted the jj/remove-bootstrap branch October 4, 2019 20:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed Request Code Review Code review requested Reviewer Can Merge If all checks have passed, non-owner can merge PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants