-
Notifications
You must be signed in to change notification settings - Fork 73
Config: Add in docs property for 1.11.3 #264
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
Conversation
This was preventing `grunt prepare` from running npm install in api.jquery.com.
Hi @xbili, thanks for your contribution. Actually, 1.11 package (and on) doesn't embed documentation anymore. So, we shouldn't add |
Also removed `docs` from `config.json`.
I see, thanks for pointing that out. I've wrapped the process into an if-else loop in the |
cwd: "tmp/api.jqueryui.com" | ||
} | ||
}, log( callback, "Done building documentation", "Error building documentation" ) ); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, I prefer you add a silent return callback and avoid the else condition (let else be the normal flow).
if ( !jqueryUi.docs ) {
return callback();
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rxaviers Sorry about that, I've made the changes.
Thank you. Please, just let me know if you need this published for your |
@rxaviers You're welcome, I think manually running |
This was preventing
grunt prepare
from running.After looking through the code again from this, the if-else loop appears to be correct. My mistake on that. But the
config.json
file seems to require adoc
property in order for the if-else loop to be executed correctly.If not
npm install
wouldn't run fortmp/api.jquery.com
. Do let me know if there are any mistakes with this!Fixes issue 262 and also this.
Thanks!