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

Namespace structure #3810

Closed
schlessera opened this issue Nov 22, 2019 · 1 comment · Fixed by #3659
Closed

Namespace structure #3810

schlessera opened this issue Nov 22, 2019 · 1 comment · Fixed by #3659
Labels
Discussion For issues that are high-level and not yet ready to implement.
Milestone

Comments

@schlessera
Copy link
Collaborator

Feature description

In #3659 we started discussing adding namespaces for new objects to start reasoning about the main architecture of v2.0 of the plugin.

We need to decide what the overall namespace structure should look like, and what conventions we want to use for new names, subnamespaces and file/older names.

We've already agreed on PSR-4 so far, so the files and folders should match the namespaces and class/interface/trait names.

What is still unclear is:

1.) Package structure in terms of overall PHP space

Instead of just going with AMP as the namespace for the plugin, we should think about the bigger picture in terms of what the PHP space might need in total for AMP support.

In that context, AMP should be the "vendor" prefix for anything related to the AMP open source project. With the usual convention of <vendor>/<package> structure as package names and root namespaces, we would then have AMP\<package> for the plugin package name and root namespace.

The <package> could be something like "AMP-WP" (in whatever notation form).

What we currently have: Amp\AmpWp.

2.) Notation or capitalization form

With PSR-4, the biggest point of discussion is usually capitalization of acronyms and trademarks. There are two basic approaches: capitalize acronyms and trademarks as they would be capitalized in regular written form, or capitalize purely on the word boundary.

Capitalizing as in written form often seems to be more correct, but can lead to difficult-to-read combinations, as there is no word spacing to keep word boundaries intact. In our case, the term "AMP WP" could for example be written as AMPWP, but that is hardly legible.

Sometimes, this is overcome by adding underscores, like AMP_WP. However this is not directly part of PSR-4 and more of a remnant of PHP < 5.3 where underscores were used to overcome the lack of namespaces. So AMP_WP would have been used in stead of AMP\WP.

Capitalizing om the word boundary only is the more popular choice in the overall PHP space, as it remove the contentious topics of acronyms and trademarks from the equation. Trademarks in particular are quite troublesome, as they might also enforce lowercase starting letters, completely breaking the PascalCase flow. Therefore, using only the word boundaries as reference makes everything more consistent and predictable and avoids discussing trademark rules. This would produce AmpWp for us.

Of course we can go with neither as well, and look only on the aesthetic part of the namespace. AmpWP was suggested as being the best-looking one. It seems to combine the word boundary approach (Amp instead of AMP) with the trademarked approach ("capital P dangit").

3.) First level of subnamespaces

The general approach in PHP is to have the most critical parts of the code that all subsystems need to use in the root namespace. This is often mostly interfaces, as that is all you should need for cross-component communication.

Then, the first level of subnamespaces should represent the subsystems of the package.

That could be something like this (random example, nothing definitive yet):

- Amp
	- AmpWp
		- Administration
		- Component
		- EmbedHandler
		- Sanitizer
		- Validation
		- Widget

As we want to plan to extract some of these subsystems into separate packages, we might already shape everything in that way to make it easy to separate later:

- Amp
	- AmpWp
	    - Administration
	    - Component
	    - EmbedHandler
	    - Widget
	- Sanitizer
	- Validation

It would regroup the WP-specific stuff, and keep the reusable, generic-PHP code separate.

We could then later easily extract these out into separate packages amp/sanitizer and amp/validation.


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

Implementation brief

QA testing instructions

Demo

Changelog entry

@schlessera schlessera added the Discussion For issues that are high-level and not yet ready to implement. label Nov 22, 2019
@westonruter
Copy link
Member

AmpWP was suggested as being the best-looking one. It seems to combine the word boundary approach (Amp instead of AMP) with the trademarked approach ("capital P dangit").

I feel this is probably the best option then. I think an acceptance criterion should be that it should feel WordPressy. Capitalizing the P ensures that. Everything under the Amp\AmpWP namespace would be influenced by WordPress requirements in any case. As we develop functionality into separate packages (e.g. #2315), these can be independent of this as you noted.

I assume that this means @package AMP should be rewritten to @package Amp\AmpWP going forward? Cf. https://github.com/ampproject/amp-wp/pull/3659/files#r349718610

@westonruter westonruter added this to the v1.5 milestone Nov 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion For issues that are high-level and not yet ready to implement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants