Skip to content

llvm-py submodule#12

Closed
jaberg wants to merge 3 commits intonumba:masterfrom
jaberg:master
Closed

llvm-py submodule#12
jaberg wants to merge 3 commits intonumba:masterfrom
jaberg:master

Conversation

@jaberg
Copy link
Copy Markdown
Contributor

@jaberg jaberg commented Jul 21, 2012

PR adds an llvm-py submodule, https submodule addressing (more firewall-friendly) and updates the README to match.

@jaberg
Copy link
Copy Markdown
Contributor Author

jaberg commented Jul 24, 2012

This looks like a duplicate of a PR by @twiecki.

Also, sorry for bundling the the DUP_TOPX and ROT_THREE patch here, I did git wrong.

@twiecki
Copy link
Copy Markdown

twiecki commented Jul 24, 2012

Yeah, I posted mine before I saw yours. Including llvm-py as a submodule is a little bit more user friendly so I think this here is better.

@jriehl
Copy link
Copy Markdown
Contributor

jriehl commented Aug 13, 2012

I'm inclined to reject this, but I'm not fully fluent with git yet. What does having a submodule buy us versus an installation dependency? What happens if a user already has the llvm-py package installed?

@teoliphant
Copy link
Copy Markdown
Contributor

I think we need to reject this. llvmpy is a dependency.

-Travis

On Aug 13, 2012, at 1:55 PM, Jon Riehl wrote:

I'm inclined to reject this, but I'm not fully fluent with git yet. What does having a submodule buy us versus an installation dependency? What happens if a user already has the llvm-py package installed?


Reply to this email directly or view it on GitHub.

@ilanschnell
Copy link
Copy Markdown
Contributor

I agree, llvmpy is a dependency, not a subproject.

@jriehl
Copy link
Copy Markdown
Contributor

jriehl commented Aug 23, 2012

Thanks for the comments. Closing.

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