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

Extract blockservice and verifcid #5296

Merged
merged 1 commit into from
Jul 28, 2018
Merged

Conversation

whyrusleeping
Copy link
Member

The extraction is steaming ahead!

License: MIT
Signed-off-by: Jeromy jeromyj@gmail.com

License: MIT
Signed-off-by: Jeromy <jeromyj@gmail.com>
@ghost ghost assigned whyrusleeping Jul 28, 2018
@ghost ghost added the status/in-progress In progress label Jul 28, 2018
@kevina
Copy link
Contributor

kevina commented Jul 28, 2018

(1) This may complicate #5285

(2) Why are we extracting verifcid? It is temporary and will go away once a more permanent solution lands which I hope to have within a month or so.

@kevina kevina self-requested a review July 28, 2018 02:47
kevina
kevina previously requested changes Jul 28, 2018
Copy link
Contributor

@kevina kevina left a comment

Choose a reason for hiding this comment

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

Unless there is a pressing need for this extraction I would appreciate it if we hold off until #5285 is figured out.

Also is vereifycid needed by someone else? It will go away once #5150 lands.

Blocking to make sure my comments are not lost in the noise. No other reason. If you wish to proceed anyway feel free to disregard my review.

@whyrusleeping
Copy link
Member Author

@kevina we can reapply the diffs as needed. This is blocking other things i'm working on. If you want, I can handle porting any conflicting patches.

@whyrusleeping
Copy link
Member Author

Why are we extracting verifcid? It is temporary and will go away once a more permanent solution lands which I hope to have within a month or so.

Thats okay, we can delete it when we finish that. See the note in the readme for that repo, it explicitly says its temporary.

@kevina
Copy link
Contributor

kevina commented Jul 28, 2018

@whyrusleeping

@kevina we can reapply the diffs as needed. This is blocking other things i'm working on. If you want, I can handle porting any conflicting patches.

Okay. For now I won't rebase #5285 as I am exploring multiple solutions and some of them might require modifying the blockservice (although I would like to avoid it, the alternative might be some nasty modifications to the gc and pinning).

> Thats okay, we can delete it when we finish that. See the note in the readme for that repo, it explicitly says its temporary.

I am not sure I understand the point then. But it doesn't cause me any problems so I don't have a problem with it.

Nevermind: I take it is needed because it is used in the blockservice.

@kevina kevina dismissed their stale review July 28, 2018 05:58

Concerns addressed.

@whyrusleeping
Copy link
Member Author

Nevermind: I take it is needed because it is used in the blockservice.

Ah, yeah. Thats the reason. Forgot to directly mention that

@whyrusleeping whyrusleeping merged commit 23f9e7b into master Jul 28, 2018
@ghost ghost removed the status/in-progress In progress label Jul 28, 2018
@whyrusleeping whyrusleeping deleted the feat/extract-blockservice branch July 28, 2018 07:48
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.

2 participants