Skip to content

Conversation

johnhaley81
Copy link
Collaborator

There was a linking exception that was fixed by changing the order of includes for node and v8 headers

There was a linking exception that was fixed by changing the order of includes for node and v8 headers
johnhaley81 added a commit that referenced this pull request Feb 24, 2016
@johnhaley81 johnhaley81 merged commit 1ab23e0 into master Feb 24, 2016
@johnhaley81 johnhaley81 deleted the fix-debug-build-windows branch February 24, 2016 20:04
@@ -1,7 +1,7 @@
// This is a generated file, modify: generate/templates/nodegit.cc.
Copy link
Collaborator

Choose a reason for hiding this comment

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

:trollface:?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's there in case somebody is looking at the generated file src/nodegit.cc and is like "why can't I commit my changes.

Copy link
Member

Choose a reason for hiding this comment

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

Should we add a Combyne comment above it?

{%-- JavaScript comment below alerts contributors to not edit the generated source --%}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Probably. That would at least clarify a bit. Might be nicer though if we just do that when we get the compiled output back, like
// This is a generated file, modify: {root template}. in the generateNativeCode.js

Looks like it's around lines 117/120 (probably a few others too)

@maxkorp
Copy link
Collaborator

maxkorp commented Feb 25, 2016

Yeah, i wish we had a good way to point to the root template in combyne so we didnt have to include that in the not generated files.

@maxkorp
Copy link
Collaborator

maxkorp commented Feb 25, 2016

#924

@johnhaley81
Copy link
Collaborator Author

👍

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