search

scijs / ndarray-blas-level2

BLAS Level 2 operations for ndarrays
MIT License
9 stars 2 forks source link

Merge tab58 prs (#4, #5, #8) #9

Closed rreusser closed 8 years ago

rreusser commented 8 years ago

There were a couple trivial conflicts, but this branch pulls in #4, #5 and #8. Plan is to tweak a couple things, merge and publish this PR, then close out the other three.

rreusser commented 8 years ago

Oops… I too blindly merged and got them in the wrong order. One sec… let me see if I can straighten this out.

rreusser commented 8 years ago

@tab58 - Okay. Final change was updating the license. I didn't make many changes to the code itself but did my best to organize things well and scaffold things well so that it hopefully scales.

I cleaned up some things test-wise. Specifically:

Maybe that's most of it.

Of course one problem here is that it could get unwieldy when we end up with a hundred files in the base repo, but we could always just move these files into lib/. Maybe that's a choice we should make sooner than later so that we can guarantee the requirability of single files.

Otherwise I'm mostly content here. Tons more work to do, but it's a much better start than I had previously.

tab58 commented 8 years ago

Hey, sorry there was so much that had to get moved in at once. I'll try to not do that again. I get a bit excited to push and I leave some things out and then it looks ugly. We can try to do this one at a time now that the files are split up into different sections.

tab58 commented 8 years ago

I downloaded welch/gencov which adds this library as a dependency. After replacing the older NPM install version with this branch, gencov's tests ran without problems. I say ship it:

              |    |    |
             )_)  )_)  )_)
            )___))___))___)\
           )____)____)_____)\\
         _____|____|____|____\\\__
---------\                   /---------
  ^^^^^ ^^^^^^^^^^^^^^^^^^^^^
    ^^^^      ^^^^     ^^^    ^^
         ^^^^      ^^^
tab58 commented 8 years ago

Should I merge this or are there issues @rreusser ?

rreusser commented 8 years ago

Sorry, yeah, let's merge! I'd be glad, but might as well confirm that you have the proper permissions. Make it v1.1.0?

Do you have an npm account for publishing?

tab58 commented 8 years ago

I don't think I've ever really published on npm. I do have an account though.

rreusser commented 8 years ago

All good. It's no trouble for me to publish but might as well get you set up. What's your username?

rreusser commented 8 years ago

Ah, added you to the npm module. Steps:

Unless I'm forgetting steps, I think that should cover it…

tab58 commented 8 years ago

Thanks for the step-by-step protocol. I'll get on it as soon as we discuss version number. It seems we're already at v1.0.2, so wouldn't we want to move to v1.1.0? I guess semver isn't explicit in what the new number needs to be so long as it's greater than the last one, so I guess we could bump to v1.5.0, but that goes against my "math grain".

Thoughts?

rreusser commented 8 years ago

Ah, oops. Yeah. my bad. You're right. 1.1.0 sounds right.

tab58 commented 8 years ago

I'm updating the package.json to reflect v1.1.0 before I merge in the PR.