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

effects: change the overlayed::Bool property to nonoverlayed::Bool #44786

Merged
merged 1 commit into from
Mar 30, 2022

Conversation

aviatesk
Copy link
Sponsor Member

@aviatesk aviatesk commented Mar 29, 2022

The current naming of overlayed is a bit confusing since
overlayed === true, which is the conservative default value, actually
means "it may be overlayed" while overlayed === false means
"this is absolutely not overlayed".
I think it should be named as nonoverlayed, and then a query name like
is_nonoverlayed would be more sensible than the alternative
is_overlayed, which actually should be something like can_be_overlayed.

@aviatesk aviatesk added the backport 1.8 Change should be backported to release-1.8 label Mar 29, 2022
@KristofferC KristofferC mentioned this pull request Mar 29, 2022
67 tasks
@Keno
Copy link
Member

Keno commented Mar 30, 2022

I think I'd keep the positive sense for method matches and method tables where the answer is strict and use nonoverlayed for the effect, (add the extra n to avoid the awkward double vowel).

@aviatesk
Copy link
Sponsor Member Author

Thanks @Keno, that sounds more reasonable. I applied all your suggestions.

@aviatesk aviatesk changed the title effects: change the overlayed::Bool property to nooverlayed::Bool effects: change the overlayed::Bool property to nonoverlayed::Bool Mar 30, 2022
The current naming of `overlayed` is a bit confusing since
`overlayed === true`, which is the conservative default value, actually
means "it _may_ be overlayed" while `overlayed === false` means
"this is absolutely not overlayed".
I think it should be named as `nonoverlayed`, and then a query name like
`is_nonoverlayed` would be more sensible than the alternative
`is_overlayed`, which actually should be something like `can_be_overlayed`.
@aviatesk aviatesk merged commit f782430 into master Mar 30, 2022
@aviatesk aviatesk deleted the avi/nooverlayed branch March 30, 2022 11:12
aviatesk added a commit that referenced this pull request Mar 31, 2022
#44786)

The current naming of `overlayed` is a bit confusing since
`overlayed === true`, which is the conservative default value, actually
means "it _may_ be overlayed" while `overlayed === false` means
"this is absolutely not overlayed".
I think it should be named as `nonoverlayed`, and then a query name like
`is_nonoverlayed` would be more sensible than the alternative
`is_overlayed`, which actually should be something like `can_be_overlayed`.
@KristofferC KristofferC removed the backport 1.8 Change should be backported to release-1.8 label May 26, 2022
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.

3 participants