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

addStatusToggle return type auf yform4 angepasst #132

Merged
merged 3 commits into from Feb 27, 2023

Conversation

@tyrant88
Copy link
Member

tyrant88 commented Feb 21, 2023

Es ist eigentlich immer eine rex_yform_list würde ich behaupten.
Weiß gerade nicht, warum das bei mir keinen Fehler geworfen hat...

@tyrant88 tyrant88 changed the title addStatusToggle Typehint wieder entfernt addStatusToggle return type wieder entfernt Feb 21, 2023
@michael-kreatif
Copy link
Contributor Author

@tyrant88 Stimmt, danke, laut Code müsste es eigentlich immer rex_yform_list sein, außer es gibt noch ein anderes addon, das den EP 'YFORM_DATA_LIST' triggert, aber das glaube ich nicht.

@tyrant88
Copy link
Member

Ich glaube das widerspräche dem Konzept der Extension Points.

@tyrant88
Copy link
Member

Ah, sehe gerade im yform changelog für Version 4:
"rex_yform_list ergänzt. Ersetzt rex_list und kann YORM Queries entgegennehmen"
Deshalb hatte Thomas Blum wohl rex_list im Code...

Copy link
Member

@tyrant88 tyrant88 left a comment

Choose a reason for hiding this comment

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

kannst/willst du das noch schnell auf rex_yform_list ändern?

@goldfoot
Copy link

Typsicherheit ist ja ansich nicht schlecht und in PHP 8 sind Union Types erlaubt. Ich schlage daher vor, beide Typen zuzulassen und den PR in "addStatusToggle return type rex_yform_list zulassen" umzubenennen:

protected static function addStatusToggle($list, $table): \rex_list | \rex_yform_list
    {
        /** @var \rex_list | \rex_yform_list $list */
       // ...
    }

@tyrant88
Copy link
Member

@goldfoot ?
Wir haben doch bereits geklärt, dass ab yform 4 (was in der package.yml vorrausgesetzt wird) der return type "rex_yform_list" ist.

Eventuell merge ich den PR so wie er ist, wenn der Ersteller ihn nicht zeitnah anpasst, deshalb bleibt der Titel erstmal. :-)

@tyrant88 tyrant88 changed the title addStatusToggle return type wieder entfernt addStatusToggle return type auf yform4 angepasst Feb 27, 2023
@tyrant88 tyrant88 merged commit 1f92bd5 into FriendsOfREDAXO:main Feb 27, 2023
@goldfoot
Copy link

@tyrant88 Stimmt, ich hatte die Beschränkung der yForm Version nicht berücksichtigt und bin daher davon ausgegangen, dass auch yForm 3 noch möglich sei.

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.

None yet

3 participants