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

Updated phpsass library to fix PHP 7 bug #21

Merged
merged 3 commits into from
Mar 1, 2018
Merged

Conversation

KZeni
Copy link
Contributor

@KZeni KZeni commented Oct 20, 2017

Fixed PHP 7 bug as mentioned by issue #19 by patching in the the latest master branch from https://github.com/richthegeek/phpsass as the version of the phpsass library that was previously being used had a PHP 7 bug.

KZeni and others added 2 commits October 19, 2017 20:50
@KZeni KZeni mentioned this pull request Oct 20, 2017
@helen
Copy link
Owner

helen commented Mar 1, 2018

@KZeni Thanks for the PR! I'm finally getting around to reviewing this. I think I will try pushing some more changes to your branch so I can get this closed up - basically, I would rather not bundle in unnecessary things such as tests and extensions.

@KZeni
Copy link
Contributor Author

KZeni commented Mar 1, 2018

@helen Ah, good point. Thanks!

@helen
Copy link
Owner

helen commented Mar 1, 2018

@KZeni I really just wanted to try this ability to push to somebody else's branch, I'm sure you would have made the changes quickly :) Anyway, something isn't working on my end but I have a feeling it's not because of this PR but because of something else, so I'm just going to merge this for now and work on the other things I need to do to ship a new release (like #2).

@helen helen added this to the 2.0 milestone Mar 1, 2018
@helen helen merged commit 4538600 into helen:develop Mar 1, 2018
@KZeni
Copy link
Contributor Author

KZeni commented Mar 1, 2018

Gotcha. Thanks again! 🎉

@KZeni
Copy link
Contributor Author

KZeni commented Jun 15, 2018

It appears this update hasn't been pushed to the WP.org plugin repository yet. Any cause for delay I can assist with?

@KZeni KZeni mentioned this pull request Nov 12, 2018
@KZeni
Copy link
Contributor Author

KZeni commented Jun 22, 2020

This fix still hasn't been uploaded to WP.org's plugin directory yet? Please, this is still causing headaches and leaves people not knowing about it already being fixed outside of WP.org thinking the plugin just doesn't work.

@helen
Copy link
Owner

helen commented Jun 22, 2020

In the time since I last replied we made a very sudden decision to move internationally and now have two young children at home on lockdown. I understand that I am the steward of this plugin and my role in the community but this is unfortunately extremely low on the priority list especially without a fix for #2.

@KZeni
Copy link
Contributor Author

KZeni commented Jun 22, 2020

I appreciate the update and sympathize with your situation.

I did make #27 just now to help out with what I can (let me know if there's anything else that could be done to help). I don't think something like PHP 7 compatibility (outright broken for most; given PHP 7's adoption) should wait any longer and/or be pushed when it comes to needing to take care of something involved like #2 before then. Ideally, this then just becomes a quick merge of that PR and then implement and tag via WP.org's SVN to address something critical like this.

There are still plenty of people out there running into this issue on the WP.org forum & elsewhere since this would be / is a great plugin to have... with or without #2 taken care of. I mean, many implement a scheme and leave it, so updating the core files after the fact might never be an issue for them while the plugin serves its full purpose, and it can be patched to take care of that later to then resolve it if/when that does come up later on in the plugin's usage (again, I don't see that holding off the plugin from not being broken with PHP 7.x in any way with it all but taken care of other than just being pushed to WP.org's SVN at this point.)

Further discussion regarding this, if any, could probably be done in #27 instead of this old PR.

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.

2 participants