-
Notifications
You must be signed in to change notification settings - Fork 56
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
I'd like to collaborate #29
Comments
Holy shit, you're pretty on the ball here, which is fantastic to see! I just moved house so I've fallen a bit behind on stuff, I'm definitely interested in catching up and working with you to get this stuff done on Gitteh. From the documentation perspective, hold up on that because I have some uncommitted stuff with a new docco system to document the JS (well, coffee-script) bridge I've written to the native bindings, so I won't need to maintain a separate fake-JS file for documentation generation. I'm at work right now, but I'll definitely get back to you ASAP! |
OK, one thing less. ;) |
+1 I would love to see this project be revived. The current "reading from a git repo" situation in Node.js is sad right now, and this was my favorite project from the past related to git+node. Let me know if you guys need any help converting from node-waf to node-gyp for the build! |
@TooTallNate thanks for the 👍, I'll [try to] get on it. Before starting working, I suggest we do a cleanup: Bundle LibGit2 (I mean, bundling, keeping a local copy along with the submodule) Move Update LibGit2 to latest version. Use only Node-GYP to build (to support latest versions) Finish refactoring the code (if it isn't done already), then remove the Stick up to the coding convention used by Google (V8) and Joyent (Node) in their code. Node.JS encourages to use classes (we're on an object-oriented language!) I suggest to use the V8U utilities to simplify wrapping (from now on). |
@samcday If you agree, |
Hey guys, still very interested in getting a new release of this out with the heavy refactoring + Node 0.8/libgit2 0.17.0 compat, most of the work is there, just needs stitching up. I moved house 3 weeks ago, and my ISP messed up pretty bad, so won't have internet until next week, so I;ll be getting back into the swing of things then ;) @TooTallNate I have some basic gyp support already, using the pull requests you sent to a couple of other projects as inspiration. It's pretty crap right now though, was trying to make use of pkgconfig on Linux, but not sure how to locate stuff in OSX, any advice/help here would be much appreciated! @jmendeth I'll finish off the refactoring, regarding most of your other points there, I think I was already moving in that direction (see the Coffeescript bridge I wrote in src/gitteh.coffee). Bundling libgit2 might make sense, though I haven't figured out the best way to do that, since libgit2 has a hard dependency on cmake, which will suck a bit for deployments to Heroku/and other PaaS systems. |
My approach on other projects has been: convert the CMake files into Gyp ones, then use them to depend on libgit2. |
@TooTallNate Do I have to include node-gyp as a dependency on |
@jmendeth Nope, npm has it bundled so when you execute |
What about older versions of NPM (such as the one bundled with Node v0.6 or even v0.4)? |
The npm with v0.6.4 and up (somewhere around there) started bundling Nobody cares about v0.4 users (in fact it's a disservice to do so), they need encouragement to update, or they can use an old version of P.S. I'm still very excited about this project getting life again! I'm using node-ffi to bind to libgit2 currently and while it works fine, I'd like for this dedicated solution to come back so I can asyncify things again (the mutexes uses in gitteh fix threading issues that async node-ffi exhibits): https://github.com/TooTallNate/n8.io/blob/1cc5ebdf32d3f1c7e08659a103691ff6934460e3/lib/git.js Cheers! |
OK. One last question: |
@jmendeth You need a "libraries" section in the binding.gyp file. See: https://github.com/rbranson/node-ffi/blob/47b6d750e98739cc12285ecf950d395a12180c7b/binding.gyp#L26-28 |
Yes, that's what I was looking for. Thanks! PS: I'm busy with Robotskirt (Sundown for Node.JS), |
I know, my "soon" isn't what you'd expect. The fact is, I'm now enough documented about I expect to be pushing to my fork quite often, |
Changes in phylosophy@samcday These changes are related to speed, comformity with other
|
Thanks for your thoughts @jmendeth, I'll do my best to address them now. To begin with 1, I definitely agree that wrapping git_oid would be a little more performant IF API users were only ever dealing with oids coming back from libgit2, but I don't think it's worth wrapping them for 2 reasons. First, I don't want to assume that API users only ever deal with refs, there is plenty of valid use cases where end users would be working with raw OIDs. Secondly, the cost of converting char* to git_old is as cheap as a #2, the reason I opted to not use V8 getter/setter callbacks in C code at all is because I actually did to begin with, and then realized that all git objects are immutable. Think about it, if you modify a git object in any way, its oid will change and thus you'll have a new object. For this reason I decided that managing the state of a git object in user space (read: the CoffeeScript bridge) is the better approach because it results in a single entry into the C code when it's time to save the object. Just a little more background on my reasoning behind #1 and #2, if you look at the oldsrc dir (soon to be deleted), you'll notice there used to be rather a lot more C code. When I picked up gitteh after a hiatus mid last year I realized that writing a lot of C code is just making the project arbitrarily complex when I could delegate most of the boilerplate to the JS, and just call into the C for the "good bits" (read: the actual libgit2 code). I've taken this to the extreme with the new CoffeeScript bridge code, by being VERY aggressive in not leaking any native code calls, or pointers to native objects, this way I can do all the painful things like validation, type checking etc in JS rather than C, which is less code and a whole lot less headaches :) #3 is a great idea, writing escaped coffeescript comments to pass through to the compiled .js file to then be parsed by a JS documentation generator written in Java was a dumb idea, and I feel stupid for not thinking of Markdown earlier. That said, I definitely do not want to use something like docco because whole "comments next to code" thing is not a good fit for a bindings library like gitteh. Do you have experience with something that might suit us better? |
I appreciate your feedback, thanks @samcday! Regarding 1.But I repeat, you probably never hardcore or input raw OIDs. Think about it: when do you see OIDs at github, other than when they are commits (a tree, a blob, a tag, etc.)? You said:
Actually, not. The OIDs are stored in memory as raw data, as it should So, it's not a
And you have the structure ready (
And you have the structure ready! Do No The APIA > data = new Buffer(20)
<Buffer 43 a9 37 ...>
> oid = new git.Oid(data)
<Oid 43a937f8f6> If for some reason a user wants to parse a HEX string: > oid = git.Oid.parse('93fe0')
<Oid 93fe000000> The rest of the OID API is intuitive as well, and won't impact the end-user experience: > oid.toString()
'93fe000000000000000000000000000000000000'
> oid.empty
false
> git.Oid.zero
<Oid 0000000000>
> git.Oid.zero.empty
true |
Regarding 2.Yeah, I know Git objects are immutable, that's why I thought of getters as a good idea; But I read your coffee bridge and you've convinced me: So, it's now official: I'll use normal properties on my rewrite, just as before. Regarding 3.Sorry, I don't get what you're asking for. Other changes
|
@jmendeth I was unaware you were pushing for a rewrite of the codebase, if that's the case then I'll probably be recommending you just maintain your own fork going forward. I'm sure we can come to some middle ground to ensure our efforts are combined, but just be aware that the current state of the C codebase and the CoffeeScript bridging code are something I arrived at after months of work, and I'm not too keen on dropping it for a "rewrite" :) Regarding #1, something you might be missing is the fact that V8 switching from JS to native C code is infinitely more expensive than anything else. It is far more expensive than a couple of memcpys, heap allocations, and conversions from v8::string to char* :) So when you have an oid object that has a constructor calling into C code, the mere switch to C code alone is going to waste a lot more CPU cycles than necessary. The approach I've been going for is that obviously calling into the C code is inevitable, but when I do it I try to make sure I get everything done in one jump. For example if I was being 100% faithful to the libgit2 API and just making the gitteh codebase a very thin wrapper (which, if I did, I might as well delete my code and recommend people use node-ffi anyway), then I would require API users to first obtain a handle to a git_oid, then a handle to a git_commit, then a handle to the git_commit parent git_oid. If the JS bridge were low level this would be 3 different JS -> C calls. With my current approach there is only one call into the C code and we do as much as we can right there and then, make sure to keep that in mind going forward! Regarding your approach with static factory methods, I'm fine if we have both construction methods available: As for the markdown, I was still hoping for something a little higher level than straight markdown authoring for documentation generation, but I'll see what it feels like to just author plain markdown for gitteh. |
Sorry for the delay. In respect to the rewrite, However, ---as you can see--- I'm doing it heavily There are some reasons why I think Gitteh needs a rewrite:
|
Regarding #1Most OIDs come from the library itself.
Bonus! No validation / conversion / parsing stuff is required. |
@samcday Just reading your last comment, I have to say:
|
@jmendeth and @samcday What's the status on each of your forks? I keep seeing that the codebase is in the middle of a refactor, for libgit2 v0.17 on both the READMEs, so it makes me hesitate to leap in and contribute on some of the little problems, like people not being able to compile it on mac OSX. Esp if things are going to be changed around. Could either one of you give us an update on the current status? it'd be much appreciated. Thanks! |
@iamwilhelm Thanks for your interest. First of all, you So it'll take long before I finally have a decent chunk of the API I should make it more clear that I'm rewriting. I'll update my README. |
@jmendeth Thanks for the answers. Are you doing the rewrite here in libgit2/node-gitteh? or are you doing it in your own fork? And if it's in your own fork, are you keeping the method signatures the same, or are you changing them around completely? |
@iamwilhelm In my own fork, of course. With respect to the signatures (JS API) there will be I'll be trying to keep the previous method names and signatures where possible. |
Ahh, I see. Is there a plan to merge the forks in the future between @jmendeth / gitteh and libgit2/gitteh? Or is that still up in the air, and remains to be seen? |
That is still up in the air, and remains to be seen. Think of my rewrite as an experiment --an experiment 2013/3/6 Wil Chung [email protected]
|
Closed for clarity and inactivity. (read: my fault) |
Let me know if I'm doing anything that's already in progress!
I'll be updating this when I complete tasks.
Fix the following tasks in
TODO
:GitObject
class which the other object types (Commit
,Tree
,Blob
, etc.) will subclass.Improve spelling, grammar and some confusing explanations at the docs.As well as add support for the following features:
A generic
Repository.get(...)
method that returns the object wrapped with the appropiate type.We're on a dynamically-typed language!
Support for
peeling
objects.Support for
revparse
(see Need some kind of high level "rev-parse" functionality. #8)Support for ODB managing and streaming (see Offer a ReadableStream/WritableStream interface to work getRawObject blobs... #4)
High-level interface to branches: a
Branch
type which subclassesReference
. Add the following methods:And some other utilities:
Support for getting libgit2 version as well as
node-gitteh
version.Determining the merge base of two or more commits.
Convenience methods for working with the HEAD and refs:
Support for
prettify
ing a Message.That's all for now ;)
Anyway, it should be great to have a wiki page that people
can edit to add and assign himself tasks, instead of a
TODO.md
.The text was updated successfully, but these errors were encountered: