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

Do not select first value as default if select value is optional #3472

Merged
merged 7 commits into from Jan 28, 2017

Conversation

Projects
None yet
3 participants
@guerler
Copy link
Contributor

commented Jan 24, 2017

Fixes #3421. This PR fixes a minor redundancy and a related bug. There is a second copy of get_initial_value inlined into the to_dict caller of some basic parameters. For the SelectToolParameter this copy is inconsistent. It picks the first option by default even if the paramater is optional while its get_initial_value caller initializes optional values with None.

@guerler guerler added this to the 17.05 milestone Jan 24, 2017

@guerler guerler added status/review and removed status/WIP labels Jan 24, 2017

@bgruening

This comment has been minimized.

Copy link
Member

commented Jan 24, 2017

@guerler shouldn't this go to 17.01 as a bugfix?
Thanks for fixing it!

@bgruening

This comment has been minimized.

Copy link
Member

commented Jan 24, 2017

@galaxybot test this

1 similar comment
@guerler

This comment has been minimized.

Copy link
Contributor Author

commented Jan 25, 2017

@galaxybot test this

@guerler guerler added status/WIP and removed status/review labels Jan 25, 2017

@bgruening

This comment has been minimized.

Copy link
Member

commented Jan 25, 2017

@guerler this should also go to 17.01, isn't it?

@guerler

This comment has been minimized.

Copy link
Contributor Author

commented Jan 25, 2017

Possibly but since this is an old minor issue, I would prefer to push it to dev first.

@guerler guerler changed the base branch from dev to release_17.01 Jan 26, 2017

@guerler guerler changed the base branch from release_17.01 to dev Jan 26, 2017

@guerler guerler added status/review and removed status/WIP labels Jan 27, 2017

@bgruening bgruening merged commit 30b4621 into galaxyproject:dev Jan 28, 2017

4 checks passed

api test Build finished. 256 tests run, 0 skipped, 0 failed.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
framework test Build finished. 137 tests run, 0 skipped, 0 failed.
Details
toolshed test Build finished. 580 tests run, 0 skipped, 0 failed.
Details
@bgruening

This comment has been minimized.

Copy link
Member

commented Jan 28, 2017

Thanks @guerler!

@guerler

This comment has been minimized.

Copy link
Contributor Author

commented Jan 28, 2017

Thanks for reporting and reviewing it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.