Skip to content

Add missing member variable in sync_model.php to fix issue #36 - #37

Merged
glynhudson merged 1 commit into
emoncms:masterfrom
davefiddes:fix-feed-list-error
Sep 3, 2023
Merged

glynhudson merged 1 commit into
emoncms:masterfrom
davefiddes:fix-feed-list-error

Conversation

@davefiddes

Copy link
Copy Markdown
Contributor

Issue #36 was caused by the $feed member variable in the Sync class not being delcared in the class. This was permitted prior to php 8.2 but will now raise a warning. The warning corrupts the JSON output preventing the function from working.

Tests:

  • Sync several feed from an upstream stable emoncms instance

Issue emoncms#36 was caused by the $feed member variable in the Sync
class not being delcared in the class. This was permitted prior to
php 8.2 but will now raise a warning. The warning corrupts the JSON
output preventing the function from working.

Tests:
 - Sync several feed from an upstream stable emoncms instance
@glynhudson

Copy link
Copy Markdown
Member

Thanks, will this be backwards compatible with PHP7?

@davefiddes

Copy link
Copy Markdown
Contributor Author

It should be fine but I haven't tested it. My change is just adding another member variable using the standard php class syntax. It seems to be just an omission and not an intentional use of a dynamic property. FWIW the rest of emoncms seems to work fine with php 8.2.

The docs for php 8.2 explain the reasoning: https://wiki.php.net/rfc/deprecate_dynamic_properties

@reedy

reedy commented Aug 27, 2023

Copy link
Copy Markdown
Contributor

Thanks, will this be backwards compatible with PHP7?

Unless something elsewhere is expecting it to be public, it is, yeah

@glynhudson
glynhudson merged commit 0d37df2 into emoncms:master Sep 3, 2023
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.

3 participants