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

complete rewrite #3

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

complete rewrite #3

wants to merge 1 commit into from

Conversation

davidmondok
Copy link
Member

So, habe dieses Plugin ebenfalls komplett umgeschrieben und Test hinzugefügt.

@@ -0,0 +1,31 @@
name: Testing DynamicContentManager
Copy link
Member

Choose a reason for hiding this comment

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

Alter plugin name

on: [push, pull_request]
jobs:
bedrock:
name: DynamicContentManager (PHP ${{ matrix.php-versions }})
Copy link
Member

Choose a reason for hiding this comment

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

Alter plugin name

${__dir}/wp.sh option update permalink_structure '/%postname%'

# Activate plugin to be developed
${__dir}/wp.sh plugin activate woda-dynamic-content-manager
Copy link
Member

Choose a reason for hiding this comment

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

Alter plugin name

Comment on lines +38 to +39
if (!$file->hasValidExtension()) {
continue;
Copy link
Member

Choose a reason for hiding this comment

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

Warum dürfen FontFiles ohne valider Extension erstellt werden. Ich bin vll noch nicht so weit im Review, dass ich den Grund erkenne ;)


public function hasValidExtension(): bool
{
$allowedExtensions = Config::loadConfigArray('valid_extensions');
Copy link
Member

Choose a reason for hiding this comment

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

So ein Funktionsaufruf machts natürlich schwieriger zu testen. Über Alternativen können wir quatschen, ist vermutlich nicht so einfach da rauszubekommen :)

Copy link
Member

Choose a reason for hiding this comment

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

Und eigentlich wichtiger, in Anlehnung an mein Kommentar weiter oben irgendwo:
Das sollte eigentlich schon beim construct passieren oder? Damit gar keine invaliden extensions angelegt werden können.

Comment on lines +52 to +55
return apply_filters(
Config::loadConfigString('filter_prefix') . 'stage_1_class',
$this->stage1Class
);
Copy link
Member

Choose a reason for hiding this comment

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

Da brauchma irgendwas, am besten eine Klasse die übern Konstruktor reinkommt. Die kennt bereits den filter_prefix, und über die wird dann das apply_filters aufgerufen. Könnte dann so ausschauen:

$this->filters->apply('stage_1_class', $this->stage1Class)


public function testFont(): void
{

Copy link
Member

Choose a reason for hiding this comment

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

Leerzeile zu viel

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