Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

cleaned up parity/signer #1551

Merged
merged 5 commits into from Jul 11, 2016
Merged

cleaned up parity/signer #1551

merged 5 commits into from Jul 11, 2016

Conversation

debris
Copy link
Collaborator

@debris debris commented Jul 6, 2016

  • cleaned up parity/signer
  • fixed compiling parity without ethcore-signer feature

@debris debris added the A0-pleasereview 🤓 Pull request needs code review. label Jul 6, 2016

pub struct SignerServer;

pub fn start(_conf: Configuration, _deps: Dependencies) -> ! {
Copy link
Collaborator

Choose a reason for hiding this comment

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

So the problem with this approach is that I'm pretty sure that file will be never modified along with on.rs (you will simply forget that you should change something here - cause we never compile without the feature).

That said, I like that approach - much cleaner, although will be difficult to maintain.

@gavofyork
Copy link
Contributor

why do we have signer as optional still?

@tomusdrw
Copy link
Collaborator

tomusdrw commented Jul 6, 2016

It's just compilation feature - like rpc, dapps and jit although some of them are enabled by default. It has to be feature cause it's dependent on rpc

@gavofyork
Copy link
Contributor

use case for rpc being optional?

@gavofyork
Copy link
Contributor

it seems there isn't one - i suggest this is refactored to make rpc and signer compiled in.

@gavofyork gavofyork added A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jul 11, 2016
@debris debris added A0-pleasereview 🤓 Pull request needs code review. and removed A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. labels Jul 11, 2016
@debris
Copy link
Collaborator Author

debris commented Jul 11, 2016

rpc and signer are no longer compile time options and are always compiled in

@gavofyork gavofyork added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jul 11, 2016
@gavofyork gavofyork merged commit c5ed363 into master Jul 11, 2016
@gavofyork gavofyork deleted the parity_signer branch July 11, 2016 15:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants