Skip to content

Add research into request creation from superglobals#3

Merged
pmjones merged 3 commits intorequest-interop:1.xfrom
nbish11:1.x
Nov 24, 2025
Merged

Add research into request creation from superglobals#3
pmjones merged 3 commits intorequest-interop:1.xfrom
nbish11:1.x

Conversation

@nbish11
Copy link
Contributor

@nbish11 nbish11 commented Nov 22, 2025

No description provided.

Copy link
Contributor Author

@nbish11 nbish11 left a comment

Choose a reason for hiding this comment

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

@pmjones
Copy link
Contributor

pmjones commented Nov 22, 2025

@nbish11 Thank you for putting that together! Some notes:

  1. It would be good to see the class and method names of the factories in question; the parameters too, if there's room.
  2. The research document is not the place for recommendations, only for clarifications on what the research itself indicates.

Once that's done, we can better consider the need (if any) for a separate "create from globals" method/interface.

@nbish11
Copy link
Contributor Author

nbish11 commented Nov 23, 2025

I will update the research document with the actual references to the names and params.

I put it all in the research document because I think it should be extremely clear as to why a createFromGlobals() method exists, why it was chosen as a separate method on a separate class, and why it goes against what the majority of projects have done.

@nbish11
Copy link
Contributor Author

nbish11 commented Nov 24, 2025

@pmjones Let me know if anything else is missing.

@pmjones
Copy link
Contributor

pmjones commented Nov 24, 2025

Thanks for the CI and Cake links, I had been working with local downloaded copies, that helps a lot.

I put it all in the research document because I think it should be extremely clear

I agree, but the place for it is in the README under Q&A. The research itself should "document findings" not "present recommendations."

@pmjones
Copy link
Contributor

pmjones commented Nov 24, 2025

Thinking about this some more: in the interest of moving this forward, I'll take it as it is, and then move the text around to the right places. Next will be adjustment of the factory class to match the research more closely.

@pmjones pmjones merged commit 21245ab into request-interop:1.x Nov 24, 2025
2 checks passed
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