Skip to content

Conversation

gnarf
Copy link
Member

@gnarf gnarf commented Jul 18, 2015

Would allow us to add a redirects.json in the following format:

{
  "/addons/": "/plugins/"
}

Closes #61

client = wp.createClient( config );

client.authenticatedCall( "jq.setRedirects", JSON.stringify( redirects ), this.async() );
});
Copy link
Member

Choose a reason for hiding this comment

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

} );?

var wp = require( "wordpress" );

grunt.registerTask( "redirects", function() {
var config = grunt.config( "wordpress" ),
Copy link
Member

Choose a reason for hiding this comment

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

We don't really do onevar anymore (jquery/contribute.jquery.org#105)

@gnarf gnarf force-pushed the redirects branch 3 times, most recently from 5c619e2 to 16714f3 Compare July 18, 2015 20:51
var redirects = grunt.file.exists( "redirects.json" ) ? grunt.file.readJSON( "redirects.json" ) : {};
var client = wp.createClient( config );

client.authenticatedCall( "jq.setRedirects", JSON.stringify( redirects ), this.async() );
Copy link
Member

Choose a reason for hiding this comment

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

Under the hood grunt.file.readJSON just reads a file and JSON.parse's it, so it seemed kinda strange that we're directly JSON.stringifying it again.

Copy link
Member

Choose a reason for hiding this comment

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

Or is this just because we can catch syntax errors in the file more easily this way?

Copy link
Member Author

Choose a reason for hiding this comment

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

Exactly, I wanted grunt to error on invalid JSON.

Copy link
Member

Choose a reason for hiding this comment

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

Fair enough.

@jzaefferer
Copy link
Member

Looks alright, along with jquery/jquery-wp-content#368

@jzaefferer
Copy link
Member

I've tested with qunitjs.com, works fine: qunitjs/qunitjs.com#103

@gnarf gnarf merged commit 81579ec into jquery:master Jul 21, 2015
jzaefferer added a commit to qunitjs/qunitjs.com that referenced this pull request Jul 21, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants