jakejs / jake

JavaScript build tool, similar to Make or Rake. Built to work with Node.js.
http://jakejs.com
Apache License 2.0
1.97k stars 190 forks source link

PublishTask namespace missing version #282

Closed SirAnthony closed 9 years ago

SirAnthony commented 9 years ago

It seems version is defined out of publish namespace, but publish task requires publish:version https://github.com/jakejs/jake/blob/master/lib/publish_task.js#L291 I did some refactoring in local copy (add customizable namespace separator basically, to support windows full paths and reduce some code duplication), and it appeared as problem in tests (all other things works fine). Is it error from the beginning, or I did something incorrectly?

mde commented 9 years ago

Yes, I didn't want to keep writing the namespace inline over and over where the prereqs get added, so I'm passing the array of task names to .map(prefixNs). You can see it right above -- all it does is prefix the 'publish' namespace on the front of each task-name.

I would love some work refactoring this -- could you make it a PR, so everyone can see it? Then we might be able to give you some direction or help.

SirAnthony commented 9 years ago

The problem is, version and release tasks defined out of publish namespace, but implied to be inside it. Not sure how it even works before. Or I really missing something.

could you make it a PR

Sure, I'm planning to do so.

mde commented 9 years ago

Ah, I see what you're talking about -- no, 'version' and 'release' task are supposed to be available from the global namespace because of backward compatibility. They just don't appear in the list of tasks for jake -T because the do not have descriptions. That's the way you create essentially a private Jake task.

SirAnthony commented 9 years ago

If it in the default namespace, why it required in publish namespace then: https://github.com/jakejs/jake/blob/master/lib/publish_task.js#L288 Also, they do have descriptions. I'm usually using content of global.jake.Task instead of jake -T.

mde commented 9 years ago

Ugh, apologies, there were commits on the release branch which hadn't made it back in to master. I've merge the release branch back, and you can see what PublishTask looks like now. I'm really sorry for the confusion.

SirAnthony commented 9 years ago

I found error in my changes, so I'm closing this. Anyway, I still have no clear vision of why publish_task work correctly, probably I missed something and need to look at it with debugger, but it is not so important now.