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

Properly handle multiple hidden datasets when populating data select options #3842

Merged
merged 1 commit into from Mar 29, 2017

Conversation

Projects
None yet
3 participants
@guerler
Copy link
Contributor

commented Mar 29, 2017

Fixes #3823.

@guerler guerler added this to the 17.05 milestone Mar 29, 2017

@guerler guerler requested a review from erasche Mar 29, 2017

@nsoranzo

This comment has been minimized.

Copy link
Member

commented Mar 29, 2017

Shouldn't this be target at release_17.01 ?

@guerler

This comment has been minimized.

Copy link
Contributor Author

commented Mar 29, 2017

Its a minor issue existing for awhile now and this fix would lead to merge conflicts due to recent tagging additions for the data selector. My plan was to fix it in dev first and then carry it backwards if necessary, but if you think its better I can target 17.01 first.

@nsoranzo

This comment has been minimized.

Copy link
Member

commented Mar 29, 2017

@guerler It is surely fine this way, especially if the backported fix is going to be different. I just wanted to double check, because merging forward is usually easier and does not require a second pull request.

Regarding if the backport is necessary at all, I'll defer to the original bug reporter @erasche.

@mvdbeek

This comment has been minimized.

Copy link
Member

commented Mar 29, 2017

Thanks @guerler, I had been very worried when I saw that happening initially.

@mvdbeek mvdbeek merged commit a0b4ab6 into galaxyproject:dev Mar 29, 2017

5 checks passed

api test Build finished. 271 tests run, 0 skipped, 0 failed.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
framework test Build finished. 140 tests run, 0 skipped, 0 failed.
Details
integration test Build finished. 25 tests run, 0 skipped, 0 failed.
Details
toolshed test Build finished. 579 tests run, 0 skipped, 0 failed.
Details

@mvdbeek mvdbeek added status/review and removed status/WIP labels Mar 29, 2017

@mvdbeek

This comment has been minimized.

Copy link
Member

commented Mar 29, 2017

@guerler This was ready for review, right? Hope I didn't merge too early.

@guerler

This comment has been minimized.

Copy link
Contributor Author

commented Mar 29, 2017

Yeah, you are good. Thanks.

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.