Skip to content

Commit 0e2dffa

Browse files
committed
Adding review changes
1 parent 71685d7 commit 0e2dffa

File tree

6 files changed

+14
-16
lines changed

6 files changed

+14
-16
lines changed

generate/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
## /generate
22

3-
The scripts and templates in this dir, help generate the source code and tests for NodeGit. The major components of generate are:
3+
The scripts and templates in this dir help generate the source code and tests for NodeGit. The major components of generate are:
44

55
1. Input
66
2. Scripts

generate/templates/README.md

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,22 +4,20 @@
44

55
## Why?
66

7-
If generated code does not accurately wrap the libgit2 calls, you might want to consider implementing manual templates in the following cases:
8-
97
#### 1. Performance
10-
> 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.
8+
> 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```.
119
1210
#### 2. Saftey
13-
> 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.
11+
> 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.
1412
1513
#### 3. Odd cases
16-
> 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.
14+
> 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``` and ```git_remote_ls```.
1715
1816
<br />
1917
-----
2018
## Implementing manual templates
2119

22-
#### 1. Copy generated .cc and .h files to *generate/templates/manual/*
20+
#### 1. Write manual .cc and .h files to *generate/templates/manual/*
2321
*.cc files -> /generate/templates/manual/src/
2422
*.h files -> /generate/templates/manual/include/
2523

lib/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
## /lib
22

3-
Contains wrappers to abstract the internals of the JavaScript part of NodeGit Module.
3+
Contains javascript extensions for the generated NodeGit modules. Any additional behavior on top of the standard libgit2 behavior will be found here.

lifecycleScripts/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
## /lifecycleScripts
22

3-
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.
3+
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.
44

55

test/README.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,17 @@
11
## /test
22

3-
Contains all the test scripts, runner and keys for running the tests.
3+
Contains all the test scripts, runner, and keys for running the tests.
44

55
-----------
66

77
#### /home
8-
contains gitconfig for the test repositories.
8+
Contains gitconfig for the test repositories.
99

1010
#### /repos
11-
contains blame, empty, nonrepo and workdir test repositories.
11+
Contains blame, empty, nonrepo, and workdir test repositories.
1212

1313
#### /tests
14-
unit tests for NodeGit.
14+
Unit tests for NodeGit.
1515

1616
#### /utils
17-
test utilities with garbage collector, index and repository setup, that can be used in tests.
17+
Test utilities with garbage collector, index, and repository setup, that can be used in tests.

utils/README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
## /utils
22

3-
contains utilities for NodeGit
3+
Contains utilities for NodeGit
44

55
#### buildFlags
6-
determines how NodeGit should build. Use `BUILD_ONLY` environment variable to build from source.
6+
Determines how NodeGit should build. Use `BUILD_ONLY` environment variable to build from source.

0 commit comments

Comments
 (0)