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

Verify dao param default vals #1769 #1824

Conversation

cyphersphinx86
Copy link

@cyphersphinx86 cyphersphinx86 commented Oct 27, 2018

Basis code for DAO param validation

New, more intuitive methods for testing min and max values of proposed DAO parameters.
Futhermore default limits set for all current DAO parameter tyeps. (Suggestions for other default values are very welcome)
Unit tests also provided.

Solves issue: #1769

    New, more intuitive methods for testing min and max values of proposed DAO parameters.
    Unit tests also provided.

    Next up will be a discussion of what limits the remaining parameters should have.

    First part of issue: bisq-network#1769
    New, more intuitive methods for testing min and max values of proposed DAO parameters.
    Unit tests also provided.

    Solves issue: bisq-network#1769
Copy link
Member

@ManfredKarrer ManfredKarrer left a comment

Choose a reason for hiding this comment

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

I could not add the comment to the code directly...

I would prefer to not have class fieds in the test as that makes the test more complex. Would prefer also to keep the formatter out completely as its an injected object. The test cannot be run atm in the IDEA without extra setup (jvm args) as the mockit framework has issues with the recent java 10 update.... I know we have other tests as well where that is the case but I prefer that new tests are as functional style as possible (e.g. simple input simple output). The formatting can be done in the caller and that does not need to be tested.

In the test there are some unused fields, please remove those.

@ManfredKarrer
Copy link
Member

As discussed on Slack there have been bigger changes of the parameter handling so that is outdated.

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.

None yet

2 participants