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

Adding docs for NodeGit #1350

Merged
merged 4 commits into from
Dec 20, 2017
Merged

Adding docs for NodeGit #1350

merged 4 commits into from
Dec 20, 2017

Conversation

mohseenrm
Copy link
Contributor

No description provided.

@@ -0,0 +1,21 @@
## /generate

The scripts and templates in this dir, help generate the source code and tests for NodeGit. The major components of generate are:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no comma after dir

If generated code does not accurately wrap the libgit2 calls, you might want to consider implementing manual templates in the following cases:

#### 1. Performance
> Everytime the library switches between the C land and the JS queue thread, there is a penalty in performance. If the generated code switches frequently, it might be better option to use manual templates.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be better:

Everytime the library switches between C land and the javascript thread, there is a penalty in performance. If in practice the usage of a method in libgit2 requires crossing the c/javascript boundary frequently, it might be better option to use manual templates. An example being Revwalk::FastWalk.

> The generated code sometimes does not handle structures that are inter-dependant. Perfect example would be convenient_hunks. Hunks references a file pointer and diff lines that are dependant on it. If persisted, it would lock the file. If garbage collected, the diff lines would cause seg fault errors. Anytime a custom solution is required, that would be hard for generated code to implement, manual templates should be used.

#### 3. Odd cases
> If a new pattern exists in libgit that would be difficult to implement using generated code, manual templates can be used for one-off cases. Typically generated code takes care of most patterns seen in libgit, but if function signatures do not follow typical pattern, manual templates could be used. Example: git_filter.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

git_remote_ls is another good example.

-----
## Implementing manual templates

#### 1. Copy generated .cc and .h files to *generate/templates/manual/*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typically, we don't have generated files to copy when we need to write manual templates.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, modified this


<br />
-----
## Implementing manual templates
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This particular list of instructions you've provided only talks about how to add a manual class/struct. What about manual functions?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't use manual functions, I'll dig around and figure out how to write and use them

@@ -0,0 +1,5 @@
## /lifecycleScripts

These scripts are responsible for downloading the right dependencies, configuring vendors, and all other dependencies that are required to build, generate and clean the module.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comma after generate

lib/README.md Outdated
@@ -0,0 +1,3 @@
## /lib

Contains wrappers to abstract the internals of the JavaScript part of NodeGit Module.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very vague, instead...

Contains javascript extensions for the generated NodeGit modules. Any additional behavior on top of the standard libgit2 behavior will be found here.

test/README.md Outdated
@@ -0,0 +1,17 @@
## /test

Contains all the test scripts, runner and keys for running the tests.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comma after runner

> Everytime the library switches between the C land and the JS queue thread, there is a penalty in performance. If the generated code switches frequently, it might be better option to use manual templates.

#### 2. Saftey
> The generated code sometimes does not handle structures that are inter-dependant. Perfect example would be convenient_hunks. Hunks references a file pointer and diff lines that are dependant on it. If persisted, it would lock the file. If garbage collected, the diff lines would cause seg fault errors. Anytime a custom solution is required, that would be hard for generated code to implement, manual templates should be used.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be better:

The generated code sometimes does not handle structures that are interdependent. An example would be git_patch and git_diff. A git_patch's memory is owned by git_diff, and that includes all of the children of git_patch, as well. So a git_diff_hunk, git_diff_line, and git_patch all are owned by a git_diff, and when that git_diff is deleted, all the memory for any patches, hunks, or lines that are in use as NodeGitWrappers are now corrupted. Further, a git_diff keeps a file handle open for its entire lifespan, which can lead to NodeGit holding onto file locks in Windows. Due to both of these compounding issues, we wrote manual templates to shift ownership away from a git_diff to git_patch, git_diff_hunk, and git_diff_line and also shorten the lifespan of a diff.


## Why?

If generated code does not accurately wrap the libgit2 calls, you might want to consider implementing manual templates in the following cases:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can lop this off and jump straight into the list.

@cjhoward92
Copy link
Collaborator

@implausible These changes look like they could be really helpful in NodeGit. Aside from the examples page, the additions seem worthwhile to me.

@MaxBittker
Copy link

nice work @mohseenrm. These would be helpful in my opinion!

@implausible implausible merged commit 022f559 into nodegit:master Dec 20, 2017
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.

4 participants