Skip to content

fix: implement upstream lookup logic for sites on php 7.4#4

Open
rivanuff wants to merge 1 commit intomasterfrom
fix/lookup-1.1.x
Open

fix: implement upstream lookup logic for sites on php 7.4#4
rivanuff wants to merge 1 commit intomasterfrom
fix/lookup-1.1.x

Conversation

@rivanuff
Copy link

@rivanuff rivanuff commented Mar 2, 2026

Alleen voor review, geen merge naar master. Hier wordt dan een legacy versie voor aangemaakt (v1.1.6)

Copy link

@mvdhoek1 mvdhoek1 left a comment

Choose a reason for hiding this comment

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

Ondanks het mierenneuken en oude bestaande code vind ik de parseURLvariables() method wel een stuk beter beter te begrijpen/lezen.

*/
private function parseURLvariables(): string
{
$arg_and = ['type:adres'];
Copy link

Choose a reason for hiding this comment

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

Mierenneuken maar snake_case vs camelCase 🐜

$arg_and = ['type:adres'];
$arg_or = [];

if (!empty($this->zip)) {
Copy link

Choose a reason for hiding this comment

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

De class properties zijn niet typed helaas maar de postcode dient altijd een string te zijn.
Een empty() check kan duiden op een code smell. Niet relevant voor deze PR maar denk dat we deze plugin later nog eens moeten checken in zijn geheel. Ik schrijf hem even op voor later.

define('GF_BAG_DIR', basename(__DIR__));
define('GF_BAG_ROOT_PATH', __DIR__);
define('GF_BAG_VERSION', '1.1.3');
define('GF_BAG_VERSION', '1.1.6');
Copy link

Choose a reason for hiding this comment

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

Ik zou de versie niet bijwerken in deze PR. Mocht iemand in de tussentijd een andere versie releasen dan ontstaat daar weer een conflict.

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