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

Make URL builder use site_url() to get correct http scheme #69

Merged
merged 3 commits into from
Jul 29, 2014

Conversation

topher1kenobe
Copy link

@westonruter could you review and let me know what you think?

@@ -505,7 +505,7 @@ static function get_cache_option_name( $src_hash ) {
static function get_dependency_minified_url( array $deps, $type ) {
$src_hash = self::hash_array( wp_list_pluck( $deps, 'src' ) );
$ver_hash = self::hash_array( wp_list_pluck( $deps, 'ver' ) );
$src = trailingslashit( get_option( 'home' ) . DIRECTORY_SEPARATOR . self::$options['endpoint'] );
$src = esc_url( trailingslashit( site_url( self::$options['endpoint'] ) ) );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@topher1kenobe I don't believe this should be esc_url this here. It will get escaped later when output.

@westonruter
Copy link
Contributor

Also fixes #50.

@westonruter
Copy link
Contributor

See also #54.

@@ -1 +0,0 @@
bin/.travis.yml
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the deletion of this symlink?

@topher1kenobe topher1kenobe merged commit 8059ee3 into master Jul 29, 2014
@westonruter
Copy link
Contributor

@topher1kenobe in think you meant to merge this on your fork instead.

@topher1kenobe
Copy link
Author

Yeah, I'm working on fixing it.

@westonruter
Copy link
Contributor

Done. I reset the branch back to c579d69.

@westonruter
Copy link
Contributor

git reset --hard c579d699e9ba9facdeee721675e51930c0c6f839
git push -f

@topher1kenobe
Copy link
Author

I did as well.

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