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.

Sign up for GitHub

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

Jump to bottom

Apply suggestions from community #10

Open

MaskyS wants to merge 3 commits into veegishx:master from MaskyS:master

Contributor

MaskyS commented Nov 16, 2019 •

edited

Loading

I've made the changes suggested in https://www.drupal.org/project/projectapplications/issues/3036543 except for:

getIpAddressOriginCountry(): Do not use cURL directly, use Drupal HTTP client API (Guzzle). That way global proxy settings and other configuration for outgoing HTTP requests are respected.

getIpAddressOriginCountry(): this will generate PHP warnings if the response is not valid JSON and you will then return a null country. Error handling is missing here.

Please test before merging.

MaskyS added 3 commits

November 16, 2019 15:45


          Rename permissions

d256547


          Use dependency injection to access services

3b2bf20


          Replace static calls and use dependency injection in form

0d42713

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

Labels

None yet