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

LAMINAS_DIACTOROS_STRICT_CONTENT_HEADER_LOOKUP should be enabled by default #203

Open
bcremer opened this issue Nov 12, 2024 · 4 comments
Open
Milestone

Comments

@bcremer
Copy link

bcremer commented Nov 12, 2024

Feature Request

Q A
New Feature yes
RFC no
BC Break yes(ish)

Summary

The Env-Variable LAMINAS_DIACTOROS_STRICT_CONTENT_HEADER_LOOKUP was introduced in 2.x to fix a bug. The fix was opt-in and was supposed to be enabled by default for 3.x. This never happened.

See: #66 (comment)

Any advice how to proceed? Which version should be targeted for that change?

@gsteel
Copy link
Member

gsteel commented Nov 12, 2024

I haven't looked at this in detail, but it looks like a probable BC break to enable it by default - since the 3.0 ship has sailed, I'll add this issue to the 4.0 milestone so it must be addressed then

@gsteel gsteel added this to the 4.0.0 milestone Nov 12, 2024
@bcremer
Copy link
Author

bcremer commented Nov 12, 2024

If it's a major version anyway I'd like to remove the LAMINAS_DIACTOROS_STRICT_CONTENT_HEADER_LOOKUP variable entirely and always apply the fix without giving an option to opt-out. What do you think?

@gsteel
Copy link
Member

gsteel commented Nov 12, 2024

I don't know enough about this right now to comment, but the dependence on an env var to fix what appears to be non-standard behaviour seems less than ideal, therefore I'd personally be in favour of getting rid of it for 4.x

There's no release branch for 4.0.x yet but that can easily be solved and it will be cut from the tip of the current 3.6.x branch when required.

cc @weierophinney, @Xerkus

@weierophinney
Copy link
Member

I'd personally be in favour of getting rid of it for 4.x

works for me!

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

No branches or pull requests

3 participants