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

Add Cookies and Queries list #1

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

alexander-schranz
Copy link
Member

@alexander-schranz alexander-schranz commented Feb 20, 2024

References

Open Questions

  • A: List of Regex or Strings?
  • B: Static methods or service with interface?
  • C: Naming Namespace, classes and methods
    • Classes:
      • Queries, QueryList, Query, ...
      • Cookies, CookieList, Cookie, ...
    • Method:
      • getList, get, Query, ...
    • Namespace:
      • PhpCmsig\HttpList, Cmsig\HttpList, ...
  • D: Composer Package Name / Repository Name
    • php-cmsig/http-list, cmsig/http-list, ...
  • E: Supported PHP Version
  • F: ...

TODO

  • Finish list (currently one list uses regex one string both should work the same way)
  • Add Coding Standard
  • Add License Header
  • Tests, Specially if we go with regexes instead of list of strings

@alexander-schranz alexander-schranz added the enhancement New feature or request label Feb 20, 2024
Copy link
Member

@Toflar Toflar left a comment

Choose a reason for hiding this comment

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

LGTM! I think we need regex because some of the names contain random numbers or identifiers.

What we imho also need then is the basic regex and correct escaping and quoting. And maybe we can provide a shortcut for Symfony's request class. It's so common that it imho qualifies for that (we can also add PSR, if anybody does the work of course).

Regarding the naming: I have no clue how to name this properly. It's a list of data you usually don't want to have on the server side :D But only relevant for caching. So maybe HttpCacheStripList 🤔

@alexander-schranz
Copy link
Member Author

@Toflar yeah I think that would make sense, would you put it into the same class all? Or seperate classes for HttpFoundation and PSR7 strip?

@Toflar
Copy link
Member

Toflar commented Feb 21, 2024

Hmm, probably in separate classes. Separation of concerns and stuff :)

@chirimoya
Copy link

chirimoya commented Feb 21, 2024

Regarding the naming: I have no clue how to name this properly. It's a list of data you usually don't want to have on the server side :D But only relevant for caching. So maybe HttpCacheStripList 🤔

It's not only about caching, e.g. the canonical url should also ignore the irrelevant query parameters.

@alexander-schranz
Copy link
Member Author

alexander-schranz commented Oct 14, 2024

I'm thinking about moving SEAL also to cms-fig namespace (but let that point discuss in Slack) What I want to discuss here is what namespace we want to use for our cooperation packages:

PHP Namespace

  • PHPCMSig\HTTPList / PHPCMSig\SEAL
  • CMSig\HTTPList / CMSig\SEAL
  • CMS\HTTPList / CMS\SEAL
  • PhpCmsig\HttpList / PhpCmsig\Seal
  • Cmsig\HttpList / Cmsig\Seal
  • Cms\HttpList / Cms\Seal

Correctly camelcased things like HTTP, CMS, SEAL would be Http, Cms, Seal so the choices would be, but I'm open here for suggestions:

  • PhpCmsig\HttpList / PhpCmsig\Seal
  • Cmsig\HttpList / Cmsig\Seal
  • Cms\HttpList / Cms\Seal

Composer / Packagist Namespace:

  • php-cmsig/http-list / php-cmsig/http-list
  • cmsig/http-list / php-cmsig/http-list
  • cms/http-list / cms/seal not sure if that is may blocked by packagist currently

Maybe @Seldaek can tell us if cms would be availabe for a cooperation namespace used as for cooperation packages between different CMS systems (currently @contao, @sulu, @TYPO3).

I personally would find Cms\Namespace and cms/package-name interresting to use for our cooperation.

@Seldaek
Copy link

Seldaek commented Oct 15, 2024

I think from my side yes I could grant you access to cms/ vendor, but cmsig would be more sensible IMO as less generic.

@alexander-schranz
Copy link
Member Author

alexander-schranz commented Oct 22, 2024

@Seldaek Thank you for the response think then we will go with cmsig.

My suggestions so would be:

PHP Namespace: CmsIg -> CmsIg\Seal, CmsIg\HttpList
Composer Namespace: cmsig -> cmsig/seal, cmsig/http-list
Plugin/Bundle/Extenstion Name: cmsig -> cmsig_seal

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants