Cleaning up to get ready for publishing to npm #129
Conversation
| @@ -0,0 +1,165 @@ | |||
| // Type definitions for Async 1.4.2 | |||
There was a problem hiding this comment.
What is this for? Part of the async package? Does it need to be in this location?
There was a problem hiding this comment.
Was this directory supposed to be submitted?
There was a problem hiding this comment.
I saw that it was included in some of our other repos, @mramato should it be included?
There was a problem hiding this comment.
Yes, for now this directory should be submitted. These are TypeScript definition files that aid Webstorm in intellisense and error checking and get updated when you update dependencies in package.json. They are discussed in my upcoming node guide. I'm hoping to improve things in the long term so that these will be handled slightly differently (and possibly not submitted) but for now this is the same thing we do in all of our other modules.
|
@lilleyse tangentially related: did you create a roadmap issue for the other cleanup for gltf-pipeline? |
|
@lilleyse I updated my guide to flesh out the publishing section some more: https://github.com/AnalyticalGraphicsInc/agi-cesium-people/tree/node-coding-guide/NodeCodingGuide#publishing-and-versioning-your-module It now has a section on the "first time" you publish. |
Not yet, I do have a running list of issues that I can create an issue for soon. |
Cool thanks, I'll check it out. |
|
Are there any other suggestions for this? |
| @@ -8,6 +8,9 @@ describe('addPipelineExtras', function() { | |||
|
|
|||
| beforeAll(function(done) { | |||
| fs.readFile(gltfExtrasPath, function(err, data) { | |||
| if (err) { | |||
There was a problem hiding this comment.
Use defined here (though I know this will eventually go away when everything is promisified.
There was a problem hiding this comment.
There are too many areas that we do this right now, I think I'm just going to save this for the cleanup.
|
Are we planning on publishing this with callbacks and the breaking backwards compatibility when we refactor for promises? Refactoring should be pretty easy and will clean up a lot of the boilerplate code, so I would recommend we do it sooner rather than later. |
|
That's all the comments I have. |
That's a good question. We still have a lot of cleanup to do #130, but at the same time we want to update the online model converter soon... @pjcozzi thoughts? |
|
@lilleyse I believe as long as you keep the version to |
|
Okay, |
|
Update the README.md since the npm scripts changed, e.g., |
|
Updated |
|
Nice! |
@mramato Would you like to take a quick look here?