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

Try to warn about all fields having inconsistent settings when they a… #33048

Conversation

hmusum
Copy link
Member

@hmusum hmusum commented Dec 17, 2024

…re used in a field set

Note that this is not done for match settings, as that will change behavior, but we should probably do that change, as it might be wrong now (e.g. you can get 'text' matching when the 2 fields in a fieldset have matching 'exact' and 'word')

@bratseth or @arnej27959 might have opinions about this

…re used in a field set

Note that this is not done for match settings, as that will change behavior,
but we should probably do that change, as it might be wrong now
(e.g. you can get 'text' matching when the 2 fields in a fieldset have
matching 'exact' and 'word')
Copy link
Member

@arnej27959 arnej27959 left a comment

Choose a reason for hiding this comment

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

This looks problematic to me. The new text claims that "matching.getType()" will be used, but the "return" that you want to remove will prevent that from happening. So we must either do the change or change how we produce the new text. Also, it doesn't warn about all fields like the commit message would indicate.

make sure that we report back the original fieldset matching
as the effective outcome for now.
@arnej27959
Copy link
Member

I changed the warning (not behavior) for now, agree that we should do more here later.

@hmusum
Copy link
Member Author

hmusum commented Dec 19, 2024

Thanks, looks good, needs a review

Copy link
Member

@arnej27959 arnej27959 left a comment

Choose a reason for hiding this comment

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

👍

@arnej27959 arnej27959 merged commit 268f85e into master Dec 19, 2024
3 checks passed
@arnej27959 arnej27959 deleted the hmusum/warn-about-all-fields-with-inconsistent-settings-when-using-fieldset branch December 19, 2024 09:15
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.

2 participants