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

Correct name of middleNames Match Score attribute in kyc-match yaml file #171

Merged

Conversation

GillesInnov35
Copy link
Collaborator

Correct name of middleNames Match Score attribute

What type of PR is this?

Add one of the following kinds:

  • correction

What this PR does / why we need it:

Correct name of middleNames Match Score response attribute.
Replace middleNamesScore by middleNamesMatchScore

Which issue(s) this PR fixes:

Fixes ##161

Special notes for reviewers:

Changelog input

 release-note

Correct name of middleNames Match Score attribute
@GillesInnov35
Copy link
Collaborator Author

hi all, this update will break backward compatibility. So could be implemented only in a major version. WDYT
BR
Gilles

@HuubAppelboom
Copy link
Collaborator

hi all, this update will break backward compatibility. So could be implemented only in a major version. WDYT BR Gilles

For KPN no problem, we have not gone live yet with customers

@fernandopradocabrillo
Copy link
Collaborator

Thanks @GillesInnov35 for the PR. We are actually integrating clients with current version, so let me check the impact with my product team.

@GillesInnov35
Copy link
Collaborator Author

thanks @fernandopradocabrillo, yes there's an impact for current implementation using this attribute. It is not the case at Orange France but it could for others countries. So let's see impact to decide then if we delay this evolution to a major version I think.
BR
Gilles

@GillesInnov35
Copy link
Collaborator Author

hi @ALL, do you think we'll correct this mistake in a future version? Thanks
Gilles

@HuubAppelboom
Copy link
Collaborator

HuubAppelboom commented Jan 23, 2025

hi @ALL, do you think we'll correct this mistake in a future version? Thanks Gilles

Sooner or later this will need to be fixed.

For KPN it is no issue to fix it now, we have not gone live yet with customers.

@ToshiWakayama-KDDI
Copy link
Collaborator

hi @ALL, do you think we'll correct this mistake in a future version? Thanks Gilles

Agree that sooner or later this will need to be fixed.

For KDDI, it is no issue to fix it now, since we have not implemented the Scoring function yet.

Thanks,
Toshi

@hdamker
Copy link
Contributor

hdamker commented Feb 4, 2025

So let's see impact to decide then if we delay this evolution to a major version I think.

@GillesInnov35 @fernandopradocabrillo please consider that you are still in the phase of initial versions with 0.x. That means that there is no assumption of backward compatibility between 0.x and 0.x+1 (cf. https://semver.org/#spec-item-4). That's the reason that we are distinct the versions for 0.x also within the server url with .../v0.x

Therefore I would say that now is good point in time to introduce such fixes.

@fernandopradocabrillo
Copy link
Collaborator

Hi all, yeah, let's fix it, we don't have impact either

Copy link
Collaborator

@fernandopradocabrillo fernandopradocabrillo left a comment

Choose a reason for hiding this comment

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

LGTM

@GillesInnov35
Copy link
Collaborator Author

hi @ToshiWakayama-KDDI , could you approve the PR if OK for you of course.
Thanks a lot
BR
Gilles

Copy link
Collaborator

@ToshiWakayama-KDDI ToshiWakayama-KDDI left a comment

Choose a reason for hiding this comment

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

Thanks a lot. Sorry for the delay.

@GillesInnov35 GillesInnov35 merged commit a1a5a87 into main Feb 10, 2025
1 check passed
@GillesInnov35
Copy link
Collaborator Author

Thanks a lot @ToshiWakayama-KDDI

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.

5 participants