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

Defer to WordPress for URLs instead of trying to guess them #185

Merged
merged 2 commits into from
May 12, 2021

Conversation

mmcev106
Copy link
Contributor

I ran into another URL issue when an alternate core directory is used. I think perhaps we should defer to WordPress for all base URLs, instead of having our own logic to generate them, since it seems to be difficult to cover all edge cases. What do you think of this approach?

@mmcev106 mmcev106 changed the title Deferred to WordPress for URLs instead of trying to guess them Defer to WordPress for URLs instead of trying to guess them Apr 26, 2021
@shadoath
Copy link
Collaborator

shadoath commented May 10, 2021

An interesting solution. It would eliminate the / issue from earlier. I like going straight to the URL.
It would need to be updated/extended for #178's added functionality.
Overall I do agree this is a better aproach.

class/class-wp-scss.php Outdated Show resolved Hide resolved
Co-authored-by: Skylar Bolton <skylar.bolton@gmail.com>
@mmcev106
Copy link
Contributor Author

I committed your suggestion to support parent themes. Do you have any other concerns we should consider before merging? I have NOT fully tested this PR. I was hoping you might have a test process that would be easy to run through...

@shadoath
Copy link
Collaborator

This repo does not yet have a good testing process to run through. I'm going to merge and pull it into some of my personal WP sites to test.

@shadoath shadoath merged commit 4f0c3dd into ConnectThink:master May 12, 2021
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