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

Allow combineDataProviders to work with custom methods using more than 2 parameters #9676

Merged
merged 1 commit into from
Feb 28, 2024
Merged

Allow combineDataProviders to work with custom methods using more than 2 parameters #9676

merged 1 commit into from
Feb 28, 2024

Conversation

karpushchenko
Copy link

@karpushchenko karpushchenko commented Feb 20, 2024 •
edited by fzaninotto
Loading

Closes #9675
The current implementation doesn't work with dataProvider methods with more than 2 parameters. According to documentation, the method parameter format is not strictly typed, so it is possible to add as many params as needed and then guess why combineDataProviders doesn't work

@fzaninotto
Copy link
Member

Thanks. Could you please add unit tests?

@karpushchenko
Copy link
Author

Thanks. Could you please add unit tests?

Yep, why not, I will update it probably tomorrow

@karpushchenko
Copy link
Author

@fzaninotto updated with a corresponding test

fzaninotto
fzaninotto requested changes Feb 22, 2024
View reviewed changes
Copy link
Member

@fzaninotto fzaninotto left a comment

Choose a reason for hiding this comment

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

Great! But this is technically a new feature, so you should PR against next rather than master.

@karpushchenko karpushchenko changed the base branch from master to next February 23, 2024 01:26
@karpushchenko karpushchenko changed the title [RFR] add use of rest parameters in combineDataProviders [WIP] add use of rest parameters in combineDataProviders Feb 23, 2024
@karpushchenko
Copy link
Author

karpushchenko commented Feb 23, 2024 •
edited
Loading

Ok, I will update it later today, not just a simple retarget of PR ( I considered it as a bugfix )

@karpushchenko
Copy link
Author

@fzaninotto, done! Sorry for the delay

@fzaninotto fzaninotto merged commit 47abf59 into marmelab:next Feb 28, 2024
11 checks passed
@fzaninotto
Copy link
Member

Thanks!

@fzaninotto fzaninotto added this to the 5.0.0 milestone Feb 28, 2024
@fzaninotto fzaninotto changed the title [WIP] add use of rest parameters in combineDataProviders Allow combineDataProviders to work with custom methods using more than 2 parameters Feb 28, 2024
@karpushchenko karpushchenko deleted the patch-1 branch February 28, 2024 07:04
@fzaninotto fzaninotto mentioned this pull request Mar 12, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fzaninotto fzaninotto fzaninotto requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
5.0.0-alpha.0
Development

Successfully merging this pull request may close these issues.

combineDataProviders does not support more than 2 parameters in custom dataProvider methods
2 participants
@karpushchenko @fzaninotto