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

updated README to include build instructions #66

Merged
merged 1 commit into from
Aug 25, 2013

Conversation

iamwilhelm
Copy link
Contributor

I've also added an to update on the location of the documentation. If new users of gitteh happen upon incorrect documentation, it's not a great first user experience.

And to update on the location of the documentation
@FrozenCow
Copy link

Yes, this is exactly what I needed. I didn't know about cake (thought the .node file was used directly).
Could cake maybe be ran as a post-install, that would make things much clearer.

@mildsunrise
Copy link
Contributor

First of all:

@samcday didn't thought on adding installation instructions,
because building Gitteh is a piece of cake. 🎉 👏


Then, about @FrozenCow's idea of calling cake on post-install,
I'll refer to the NPM documentation:

NOTE: INSTALL SCRIPTS ARE AN ANTIPATTERN

tl;dr Don't use install. Use a .gyp file for compilation, and prepublish for anything else.

You should almost never have to explicitly set a preinstall or install script. If you are doing this, please consider if there is another option.

@FrozenCow
Copy link

Ok, alright. I just didn't know of cake and npm was obvious for me, that's why I suggested 'some' post-install way to do that. If that's not recommended, the readme is fine too.

@mildsunrise
Copy link
Contributor

As the docs recommend, cake should be in prepublish instead,
so that the NPM artifact you download is already built.

Now if you want to develop Gitteh, you should clone the repo
and build (and rebuild many times during development) according
to the README's instructions. :)

It would make no sense to have cake on install, because you'd still
have to run it when you modify the sources.

@FrozenCow
Copy link

Yes indeed, that makes sense. Thanks for enlightening me.

@iamwilhelm
Copy link
Contributor Author

I find that explicit build instructions help a lot in getting people onboard. @samcday should I make changes as @FrozenCow suggested or other changes?

@lushzero
Copy link

@iamwilhelm Having spent a week futzing with the source before realizing the cake file was vital I'm just glad to see that documented somewhere. I'd be happy to work on documentation and examples if someone can throw together a really messy set of example code that is known to work and is correct as to the goals and best practices of this module. Right now it seems really a mess, the docs don't match the code which doesn't match what limited examples there are (that don't seem to work) and there are several aspects of the codebase that seem bitrotted with unused files and dead ends/functions.

I completely agree that a straightforward build process is vital to helping new contributors. Is there a reason no to create a .gyp file and move to that? I'm not that familiar with coffeescript but isn't it sort of over the top in it's use here. It's the end of a process that builds a javascript binding module to a C library with a C++ bridge with coffeescript macros to generate a javascript bindings file. Is there a reason coffee script shouldn't just be cut out of that or am I missing what it adds here?

@FrozenCow
Copy link

@lushzero I can see why the coffeescript is there. It mostly validates the input parameters on their type and puts the output (callback) in some nicer form. These kind of stuff is indeed annoying to implement in C++, so I can see why some form in-between gitteh.node and the user is needed.

It seems coffeescript is used because it looks a bit more concise, like for defining classes.

@iamwilhelm
Copy link
Contributor Author

@lushzero Last I heard, @samcday was in the middle of documenting, and I see skeleton outlines in the docs/, but I don't know if he's generating them or writing them by hand. The only way I figured out what the method signatures were, was partially from the old docs, but with async callbacks only, and looking at the gitteh.coffee source, to figure out that all the repository.getTree(), repository.getCommit(), etc calls are now just repository.tree() and repository.commit() calls. Do the examples in examples/ not work? I haven't tried them.

As to the build and gyp. I'm not a maintainer, so someone else, like @samcday can probably better answer that.

@mildsunrise
Copy link
Contributor

@lushzero I can see why the coffeescript is there. It mostly validates the input parameters on their type and puts the output (callback) in some nicer form. These kind of stuff is indeed annoying to implement in C++, so I can see why some form in-between gitteh.node and the user is needed.

It seems coffeescript is used because it looks a bit more concise, like for defining classes.

I'll answer that:

@samcday tried to make the C++ wrapper layer as thin as possible.
That is, do what is strictly necessary on the native side, and the rest on
the JS side (in that case, in CoffeeScript).

According to @samcday, the oldsrc folder contained the first try at Gitteh,
which did almost everything in the C side, and then a refactoring was started
to move as many code as possible to the JS side.

Why do that? Apart from the anonying point, the switch from JS to C++ is
very slow and takes too many CPU cycles.

For more info, you can look at @samcday's comments at this thread.

@lushzero
Copy link

Just to clarify my comment wasn't to say there isn't a need for something on the JS side, validation, form, etc. It just seems to me it is another layer of obfuscation on an already complicated workflow to have that been in coffeescript instead of straight JS. Is @samcday out of the picture at this point? Seems like there are several contributors circling and there haven't been any commits in over a month. Based on what I've read it seems the project is frozen indefinitely at this point.

@mildsunrise
Copy link
Contributor

It just seems to me it is another layer of obfuscation on an already complicated workflow to have that been in coffeescript instead of straight JS.

I agree. Coffeescript is beautiful, but it just complicates the workflow even more.

Is @samcday out of the picture at this point? Seems like there are several contributors circling and there haven't been any commits in over a month. Based on what I've read it seems the project is frozen indefinitely at this point.

True, neither @samcday nor @ben have said anything for a long time.

@FrozenCow
Copy link

Maybe @samcday has unsubscribed to notifications or maybe he's on holiday. Whatever the case, I've send him an email just to be sure, hopefully he won't mind.

@mildsunrise
Copy link
Contributor

Even if @samcday unsubscribed he would still get emails from all these @ mentions! He may be on holyday.

@samcday
Copy link
Contributor

samcday commented Aug 25, 2013

Hi gang (@lushzero , @jmendeth, @FrozenCow, @iamwilhelm)

Sorry for letting this thread languish for so long without a response. I've been in the process of moving states for an exciting new job and haven't had a whole lot of time for anything but packing, cleaning, and family-time ;)

@iamwilhelm Thanks a lot for the PR, the README was definitely in need of an update.

Regarding the older cruft lying around in the repo and the state of documentation right now, fairly recently I was working on inline-documenting the CoffeeScript bindings (as they represent 100% of the public API) and then outputting it all purdy using Sphinx and the coffeescript domain in the sphinx-contrib repo. I'm most of the way thru this, so expect a new version of the documentation shortly.

@jmendeth thanks for holding the fort and replying to quickly on this stuff, everything you said was 100% correct. Also, thanks for mentioning that prepublish npm script, I was looking for something like that a while ago to generate the lib/gitteh.js and never managed to dig anything up.

@lushzero I understand your frustrations regarding gitteh, as I run into projects every day that seem to do what I want, but aren't as documented / clean as I'd like. However, regarding your comments about contributors circling and the current "indefinitely frozen" state of gitteh, I would note that people who take the time to complain about the inactivity of a project could maybe spend that time actually contributing to the project ;)

Also, regarding your confusion as to whether or not CoffeeScript merits the additional complexity for this project, I think you have a good point. At the time that I undertook the gitteh rewrite I was slingin' a lot of CS, so it seemed like a logical approach at the time. Personally, I still think it makes sense but if the general consensus is that it makes the codebase too confusing then we can certainly look at alternatives. I think it's worth noting that if the CS bridge is too friction-y as a result of extra commands that need to be run, we could easily have the development (cloned repo) mode of gitteh use the CS source unit directly, rather than requiring cake build (but of course you checked the Cakefile and you're using cake watch which makes things easier, right? :D)

samcday pushed a commit that referenced this pull request Aug 25, 2013
updated README to include build instructions
@samcday samcday merged commit 11ece48 into libgit2:master Aug 25, 2013
@iamwilhelm
Copy link
Contributor Author

However, regarding your comments about contributors circling and the current "indefinitely frozen" state of gitteh, I would note that people who take the time to complain about the inactivity of a project could maybe spend that time actually contributing to the project ;)

If we'd rather people contribute than complain, then we have to make it easier for them to contribute than complain. This means that:

  1. Master should always be compiling and passing all the tests. A radical rewrite should be in a separate branch. Pre 0.17.x I had wanted to dive into contributing, but I didn't want to contribute code that was just going to be wiped out.

  2. Documentation should be up to date, at the very least, the generated API docs. That not only make it easier for new users to get started, but it also gives a singular place for potential contributors to hang their mental hat on, as a starting point for exploring the code. It's not always immediately apparent just from the code, which place is the interface, and which place is the plumbing.

  3. Documentation should explicitly spell out how to setup the project, compile the project, and run the project. This is along the lines of, the easier we make it to get started, the more people will contribute.

@samcday
Copy link
Contributor

samcday commented Aug 25, 2013

@iamwilhelm You're spot on. I am keen on making gitteh easier to hack on to encourage contributors.

Regarding point 1, the 0.17 refactor was definitely handled wrong. I learnt some lessons from that and will ensure I don't make the same mistake in future for other projects. For gitteh specifically I can assure you no major refactors are planned in any kind of time frame :)

Documentation is definitely important, and I understand what you mean about documentation targeting not just end users but would-be contributors as well. I plan on getting the new documentation I was working on recently out the door this weekend, I'll make sure to write up some information about how the project is structured, and how to get it up and running in a development setting quickly.

@iamwilhelm
Copy link
Contributor Author

@samcday

Sounds good. I look forward to the changes, and I'll contribute documentation as I can, to get more familiar with the code base.

@mildsunrise
Copy link
Contributor

👍 I'll do that as well; I'm quite lost since the refactor.
Maybe we should assign parts to avoid duplicate work.
Time will arrive, I'm curious to see how
sphinx-coffeescript looks! :3

@iamwilhelm
Copy link
Contributor Author

Oh, and regarding Coffeescript, I tend to like using it. But then again, I haven't worked deeply with the gitteh source, so the people that use it more day to day should have more of a say.

@mildsunrise
Copy link
Contributor

I also like to use CoffeeScript. What worries us is that it's potentially adding complexity
--especially to new contributors which may not even know CoffeeScript.

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.

5 participants