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.

Sign up for GitHub

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

Jump to bottom

Add a flag for enabling experimental features #2056

Merged
merged 2 commits into from
Feb 10, 2017
Merged

Add a flag for enabling experimental features #2056

merged 2 commits into from
Feb 10, 2017

Conversation

str4d
Copy link
Contributor

@str4d str4d commented Jan 31, 2017

Closes #2035.

@str4d str4d added this to the 1.0.6 milestone Jan 31, 2017
@daira
Copy link
Contributor

daira commented Feb 8, 2017

I'm assuming this should not be reviewed yet because it has [WIP].

@str4d str4d changed the title [WIP] Add a flag for enabling experimental features Add a flag for enabling experimental features Feb 8, 2017
@str4d
Copy link
Contributor Author

str4d commented Feb 8, 2017

@daira ready for review, now that #2035 (comment) is addressed.

@bitcartel
Copy link
Contributor

utACK

@bitcartel bitcartel added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 9, 2017
@bitcartel bitcartel assigned bitcartel and str4d and unassigned bitcartel Feb 9, 2017
daira
daira requested changes Feb 9, 2017
View reviewed changes
src/wallet/rpcwallet.cpp
@@ -2002,7 +2002,7 @@ Value encryptwallet(const Array& params, bool fHelp)
if (!EnsureWalletIsAvailable(fHelp))
return Value::null;

auto fEnableWalletEncryption = GetBoolArg("-developerencryptwallet", false);
auto fEnableWalletEncryption = fExperimentalMode && GetBoolArg("-developerencryptwallet", false);
Copy link
Contributor

Choose a reason for hiding this comment

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

This silently ignores -developerencryptwallet when not in experimental mode. It should give an error.

Copy link
Contributor

Choose a reason for hiding this comment

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

Otherwise ut(ACK+cov).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, you mean there should be an error saying "please enable -experimentalfeatures".

Copy link
Contributor

@daira daira Feb 9, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Given the check here, this change is no longer required. Does not block.

@bitcartel
Copy link
Contributor

@str4d Minor change requested in review from @daira

@str4d
Copy link
Contributor Author

str4d commented Feb 9, 2017

@bitcartel @daira Amended commit to add an init check.

@zkbot
Copy link
Contributor

zkbot commented Feb 9, 2017

☔ The latest upstream changes (presumably #1965) made this pull request unmergeable. Please resolve the merge conflicts.

@bitcartel
Copy link
Contributor

ACK

@bitcartel
Copy link
Contributor

@zkbot r+

@zkbot
Copy link
Contributor

zkbot commented Feb 9, 2017

📌 Commit b8eb377 has been approved by bitcartel

@zkbot
Copy link
Contributor

zkbot commented Feb 9, 2017

⌛ Testing commit b8eb377 with merge 7d4ced9...

zkbot added a commit that referenced this pull request Feb 9, 2017
@zkbot
Auto merge of #2056 - str4d:2035-experimental-mode, r=bitcartel
7d4ced9 
Add a flag for enabling experimental features

Closes #2035.
@zkbot
Copy link
Contributor

zkbot commented Feb 10, 2017

☀️ Test successful - zcash

@zkbot zkbot merged commit b8eb377 into zcash:master Feb 10, 2017
@str4d str4d deleted the 2035-experimental-mode branch January 27, 2023 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@daira daira daira approved these changes

@ebfull ebfull Awaiting requested review from ebfull

Assignees

@str4d str4d

Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Milestone
1.0.6
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@str4d @daira @bitcartel @zkbot