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

introduce configuration variable for services #11

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

krabina
Copy link
Contributor

@krabina krabina commented Jun 19, 2020

in reference to #5
new configuration parameter $wgShariffServices introduced. Users can override default values with their own desired services.

Bernhard Krabina added 4 commits June 19, 2020 10:29
implement service configuration variable
make use of new config variable
documentation reflecting configuration option
@krabina krabina changed the title Patch 4 introduce configuration variable for services Jun 19, 2020
@krabina
Copy link
Contributor Author

krabina commented Jul 31, 2020

any news on this?

@krabina
Copy link
Contributor Author

krabina commented Dec 4, 2020

Hello @vonloxley - lockdowns are a good time to accept PRs :-)

@vonloxley
Copy link
Owner

No lockdown for me, only work.

@vonloxley
Copy link
Owner

Thank you very much for this pull-request. I like the new functionality.
While I’m sure, that simply writing the service-names into data-services, the documentation states, that it should be An entity-encoded JSON string containing an array of service names to be enabled. Can you please make sure, that the user-input in the new config variable is converted into the required format?

Besides, as I use this code in production, I’d appreciate it, if none of the other defaults (like data-button-style) are changed.

@vonloxley vonloxley marked this pull request as draft April 25, 2021 11:04
@vonloxley vonloxley linked an issue Apr 25, 2021 that may be closed by this pull request
@krabina
Copy link
Contributor Author

krabina commented Apr 29, 2021

hmm. To be honest, I don't really know what to do. Before the PR it was set fixed to "twitter,facebook,linkedin,print,info", which is not a json string either. I am not really fluent in PHP, so I don't know how to improve this.

Also, I am not messing with data-button-style afaik, so I also don't know what to do with this requirement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unclear how to enable/disable services
2 participants