Factor out base class for structs / classes with cType#983
Merged
johnhaley81 merged 1 commit intonodegit:masterfrom Apr 11, 2016
Merged
Factor out base class for structs / classes with cType#983johnhaley81 merged 1 commit intonodegit:masterfrom
johnhaley81 merged 1 commit intonodegit:masterfrom
Conversation
srajko
force-pushed
the
wrapper-base-class
branch
9 times, most recently
from
7e88272 to
daa9317
Compare
- Separated cpyFunction from dupFunction in descriptor (since semantics are different - only affects git_oid)
- Added a {{ cppFunctionName }}Traits class to start leaning more on C++ templates where possible
- Assigning `free` as `freeFunctionName` for structs (since that was the previous behavior)
- Extracting a number of variables and methods from class_ and struct_ combyne templates into a base class (mostly dealing with construction / destruction and memory management). This is to extend the memory management coverage that was implemented for class_ types to struct_ types. I tried to maintain a similar .h / .cc separation that we had before.
srajko
force-pushed
the
wrapper-base-class
branch
from
daa9317 to
e15808a
Compare
srajko
changed the title
Collaborator
|
👍 |
Member
|
I much prefer keeping Combyne usage to a minimal, 👍 for language features. |
Collaborator
|
A lot of it is navigating the treacherous world of "What does |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Starts towards more code sharing between the struct_* and class_* combyne templates - I needed this to make the new memory management code available to structs (it was only implemented for classes initially). There is probably more code that could be pulled in down the line but I wanted to keep this minimal for what I needed.
This leans more heavily on C++ templates than NodeGit code typically does, so this is something to consider - if needed I can probably get similar code sharing via combyne partial templates but I'd prefer to use language features when reasonable. v8 and Nan do use C++ templates / traits classes in similar ways, so at least it's not a completely new pattern to the project.
See code / commit comments for more details on the changes.