Automattic / pym-shortcode

A WordPress solution to embed iframes that are responsive horizontally and vertically using the NPR Visuals Team's https://blog.apps.npr.org/pym.js/
https://wordpress.org/plugins/pym-shortcode/
GNU General Public License v2.0
14 stars 7 forks source link

Fix http strings to https where possible #77

Open hassanelnajjar opened 3 years ago

hassanelnajjar commented 3 years ago

search for http: and check the url using chrome and then change the secure url to https

relates #73

Changes

This pull request makes the following changes:

search for http: and check the url using chrome and then change the secure url to https

Why

For #

Testing/Questions

Features that this PR affects:

Questions that need to be answered before merging:

Steps to test this PR:

Additional information

INN Member/Labs Client requesting: (if applicable)

benlk commented 3 years ago

I'm no longer a maintainer on this project, so the folks at Automattic can overrule me on this, but:

In js/pym.js, you do change one http to https, but you also changed the whitespace and formatting of the entire file. That's not optimal, especially since js/pym.js is a file that was copied in directly from a third party. Can you revert the whitespace changes, please?

hassanelnajjar commented 3 years ago

I'm no longer a maintainer on this project, so the folks at Automattic can overrule me on this, but:

In js/pym.js, you do change one http to https, but you also changed the whitespace and formatting of the entire file. That's not optimal, especially since js/pym.js is a file that was copied in directly from a third party. Can you revert the whitespace changes, please?

I fixed the white spaces :)

benlk commented 3 years ago

This patch looks much cleaner now; thank you!