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

fix(duplicate source): renamed logical index param #452

Merged
merged 1 commit into from May 16, 2022

Conversation

dmgauthier
Copy link
Contributor

@dmgauthier dmgauthier commented May 10, 2022

Acceptance Criteria

I decided to change only the params at the moment of the call and not the way we use the method in order to avoid making a breaking change. Tell me if you think I should have done it otherwise.

EDIT: that param while duplicating a source never worked. But the API doesn't throws so we learned it recently.

  • JSDoc annotates each property added in the exported interfaces
  • The proposed changes are covered by unit tests
  • Commits containing breaking changes a properly identified as such
  • README.md is adjusted to reflect the proposed changes (if relevant)

@dmgauthier dmgauthier requested a review from a team as a code owner May 10, 2022 16:49
Copy link
Member

@toofff toofff left a comment

Choose a reason for hiding this comment

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

I think you could directly change the name of the parameter, you don't change its nature or even its position, you just change the name, it shouldn't be a breaking change

@dmgauthier dmgauthier force-pushed the fix-ADUI-7947-logicalindex-param-renamed branch 2 times, most recently from eca6100 to 9fbb3c9 Compare May 12, 2022 13:58
@louis-bompart
Copy link
Contributor

@dmgauthier, the conflict will prolly be auto-resolved once you merge it on your machine, given that's only some EoL issue from what I see.

@dmgauthier dmgauthier force-pushed the fix-ADUI-7947-logicalindex-param-renamed branch from 9fbb3c9 to 6ed2bbe Compare May 16, 2022 18:08
@dmgauthier dmgauthier merged commit 9967ed6 into master May 16, 2022
@dmgauthier dmgauthier deleted the fix-ADUI-7947-logicalindex-param-renamed branch May 16, 2022 18:15
@coveobot
Copy link

🎉 This PR is included in version 30.5.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants