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

Fix npm2 support #18

Merged
merged 3 commits into from Dec 9, 2014
Merged

Fix npm2 support #18

merged 3 commits into from Dec 9, 2014

Conversation

rmg
Copy link
Member

@rmg rmg commented Dec 9, 2014

Problem: npm run build fails on npm2 if there is no build run-script defined.
Fix: Only run npm run build step if there is a build run-script defined.

The fix is easy, but I'm going to spend a few minutes trying to get CI green if I can.

As of npm 2.1.8, npm run will exit with an error if the run script is
not defined in package.json.

Fixes #17
@rmg rmg added the #review label Dec 9, 2014
@@ -91,6 +92,7 @@ exports.syncBranch = function gitSyncBranch(gitDstBranch) {
var src = resolveBranch('HEAD');
var dst = resolveBranch(gitDstBranch);

debug('syncing dst(%p) with src(%o)', src, dst);
Copy link
Contributor

Choose a reason for hiding this comment

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

%p %o, what do those do?

Copy link
Member Author

Choose a reason for hiding this comment

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

%p was a typo. %o is a new feature in debug@2 for util.inspect format, which is sometimes easier on the eyes than %j's JSON formatting.

@sam-github
Copy link
Contributor

Would be nice to have green CI, yes.

@rmg
Copy link
Member Author

rmg commented Dec 9, 2014

test please

@rmg
Copy link
Member Author

rmg commented Dec 9, 2014

And we have a party. CI fix was partially a Jenkins config change and partially switching to strong-fork-syslog so 0.11.x tests pass.

@sam-github PTAL.

@rmg rmg assigned sam-github and unassigned rmg Dec 9, 2014
@rmg
Copy link
Member Author

rmg commented Dec 9, 2014

@sam-github I can remove the debug commit(s) before merge, if you like, but the extra debug might be useful.

@@ -91,6 +92,7 @@ exports.syncBranch = function gitSyncBranch(gitDstBranch) {
var src = resolveBranch('HEAD');
var dst = resolveBranch(gitDstBranch);

debug('syncing dst(%o) with src(%o)', src, dst);
Copy link
Contributor

Choose a reason for hiding this comment

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

%o and %p are not documented console.log flags. http://nodejs.org/api/util.html#util_util_format_format What are they from?

Copy link
Contributor

Choose a reason for hiding this comment

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

debug-js/debug#160, for the record

@sam-github
Copy link
Contributor

debug statements are good, I just don't understand that one.

other than that, LGTM

@sam-github sam-github assigned rmg and unassigned sam-github Dec 9, 2014
@rmg
Copy link
Member Author

rmg commented Dec 9, 2014

Correction, was actually added in debug 1.0.0.. though that changelog entry is the only documentation.

Useful for debugging tests, such as discovering that the way Jenkins
was configured to run the tests from a detached HEAD completely broke
the git build process.
A fork of node-syslog with better support for node v0.11.x
@rmg rmg merged commit ad9b7f4 into master Dec 9, 2014
@rmg rmg removed the #review label Dec 9, 2014
@rmg rmg deleted the fix/npm2-support branch December 9, 2014 19:47
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.

None yet

2 participants