search

jackjamieson2 / yarns-microsub-server

Yarns is a Microsub server that runs as a plugin on your WordPress site.
GNU General Public License v2.0
20 stars 4 forks source link

Use get_option() instead of get_site_option() #106

Closed florianbrinkmann closed 3 years ago

florianbrinkmann commented 3 years ago

The plugin uses get_site_option() to get option values, but update_option() to set it. That does not work on multisites, this PR fixes that by replacing update_option() with update_site_option().

jackjamieson2 commented 3 years ago

Thanks for this! I’ll review and merge this as soon as I can (alongside a few other long needed dependency updates etc) On Dec 19, 2020, 20:11 +0900, Florian Brinkmann notifications@github.com, wrote:

The plugin uses get_site_option() to get option values, but update_option() to set it. That does not work on multisites, this PR fixes that by replacing update_option() with update_site_option(). You can view, comment on, or merge this pull request online at: https://github.com/jackjamieson2/yarns-microsub-server/pull/106 Commit Summary

• Replace update_option() with update_site_option()

File Changes

• M includes/class-yarns-microsub-admin.php (4) • M includes/class-yarns-microsub-aggregator.php (6) • M includes/class-yarns-microsub-channels.php (14) • M yarns-microsub.php (6)

Patch Links:

• https://github.com/jackjamieson2/yarns-microsub-server/pull/106.patch • https://github.com/jackjamieson2/yarns-microsub-server/pull/106.diff

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

florianbrinkmann commented 3 years ago

You’re welcome! I just started thinking if it maybe would be better to use get_option() and update_option(), so that on a multisite one could have different channels and sites per site.

florianbrinkmann commented 3 years ago

I just updated the PR, I guess it makes more sense this way.

jackjamieson2 commented 3 years ago

Hi @florianbrinkmann Sorry I totally lost track of this!

@dshanske, since you're making a few changes, do you know of any potential conflicts this could introduce?

florianbrinkmann commented 3 years ago

No problem @jackjamieson2!

dshanske commented 3 years ago

There's no potential conflict yet. I haven't dug down into anything. So merging isn't an issue