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

Interact Menu - Add nil handling for condition #9922

Merged
merged 3 commits into from
Apr 7, 2024

Conversation

johnb432
Copy link
Contributor

@johnb432 johnb432 commented Apr 4, 2024

When merged this pull request will:

  • Title.
  • If any other type than nil or bool is returned, it will throw an error.

IMPORTANT

  • If the contribution affects the documentation, please include your changes in this pull request so the documentation will appear on the website.
  • Development Guidelines are read, understood and applied.
  • Title of this PR uses our standard template Component - Add|Fix|Improve|Change|Make|Remove {changes}.

@johnb432 johnb432 added the kind/enhancement Release Notes: **IMPROVED:** label Apr 4, 2024
Copy link
Contributor

@Dystopian Dystopian left a comment

Choose a reason for hiding this comment

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

I suppose this if after #9876. Then this PR can break some 3rd party stuff (if any) where nil is returned in conditions. For BWC we should treat nil return as true instead. But I'm personally vote for PR as it is.

@johnb432
Copy link
Contributor Author

johnb432 commented Apr 4, 2024

I suppose this if after #9876. Then this PR can break some 3rd party stuff (if any) where nil is returned in conditions. For BWC we should treat nil return as true instead. But I'm personally vote for PR as it is.

It's an error from the user, so imo BWC is not a concern here.

Co-authored-by: PabstMirror <pabstmirror@gmail.com>
[]
};

// Return nothing if the action is to far (including checking sub actions) [DISABLED FOR NOW ref #2196]
// Return nothing if the action is too far (including checking sub actions) [DISABLED FOR NOW ref #2196]
Copy link
Contributor

Choose a reason for hiding this comment

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

Yikes.

Big yikes.

@LinkIsGrim LinkIsGrim merged commit f5e8e06 into master Apr 7, 2024
5 checks passed
@LinkIsGrim LinkIsGrim deleted the interact-menu-nil-condition-check branch April 7, 2024 01:11
@PabstMirror PabstMirror added this to the 3.17.1 milestone May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Release Notes: **IMPROVED:**
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants