-
Notifications
You must be signed in to change notification settings - Fork 335
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
Cleaner enabling of development service #260
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JoshOrndorff
added
A0-pleasereview
Pull request needs code review.
B5-clientnoteworthy
Changes should be mentioned in any downstream projects' release notes
C3-medium
Elevates a release containing this PR to "medium priority".
labels
Feb 22, 2021
JoshOrndorff
added
the
A1-needsburnin
Pull request needs to be tested on a live validator node before merge.
label
Feb 22, 2021
crystalin
approved these changes
Feb 23, 2021
JoshOrndorff
commented
Feb 23, 2021
4meta5
approved these changes
Feb 23, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
A0-pleasereview
Pull request needs code review.
A1-needsburnin
Pull request needs to be tested on a live validator node before merge.
B5-clientnoteworthy
Changes should be mentioned in any downstream projects' release notes
C3-medium
Elevates a release containing this PR to "medium priority".
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Solves #231
This PR continues to improve the development service from both architectural and UX perspectives.
Reduced Code Duplication
On the architectural side, this further reduces duplicated code between the development and full services. There is are no longer separate files for the two services. The two services now share their
native_executor_instance!
invocation and their entirenew_partial
functions.As a side benefit, this also works around an issue I discovered with manual seal where we weren't properly checking inherents paritytech/substrate#8164 That's the issue that caused me to incorrectly conclude that the initial pass at the author filtering was working as expected.
This is still not the end of the road for service code de-duplication. There is more improvement possible, that will come if future PRs.
UX Improvements
This allows users to explicitly request the dev service by using a dedicated
--dev-service
flag. We are now no longer abusing the--dev
flag, so its role can return to being a quick shortcut flag that uses all the right defaults for a simple local one-node network.We also allow chain specs to request the dev service by using the string
"dev-service"
in therelay_chain
field.Checklist