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

feature: refactor land into a seperate modules #86

Closed
refack opened this issue Jan 27, 2017 · 9 comments
Closed

feature: refactor land into a seperate modules #86

refack opened this issue Jan 27, 2017 · 9 comments
Assignees

Comments

@refack
Copy link

refack commented Jan 27, 2017

nodegit is a dependency monster (barly buildable on windows), and provides a very specific function, that's not nececerily a main use-case.

@refack refack changed the title feature: refactor git into a seperate modules feature: refactor land into a seperate modules Jan 27, 2017
@alangpierce
Copy link
Member

Yeah, I've seen various problems with nodegit, e.g. #67. :-/

nodegit is used in convert as well, so it's not trivial to just isolate it. But it may be best to rewrite both convert and land to just shell out to git (which is the approach I originally took for a while).

@refack
Copy link
Author

refack commented Jan 27, 2017

There are thin-wrappers, such as git-js

@alangpierce
Copy link
Member

Ooh, cool, thanks, I hadn't seen that.

@refack
Copy link
Author

refack commented Jan 27, 2017

Would you mind if I give it a try (i.e. assign to me)?

@alangpierce alangpierce assigned alangpierce and refack and unassigned alangpierce Jan 27, 2017
@alangpierce
Copy link
Member

alangpierce commented Jan 27, 2017

@refack sure, done. The previous code in 8c78da9 has the original code from before I switched convert to nodegit. Also may be good to do things incrementally, like switch convert (or part of convert) to use git-js first. convert also has plenty of tests, and unfortunately land doesn't have tests at the moment because they were hard to write.

Thanks!

@alangpierce
Copy link
Member

@refack FYI I just moved this project to the decaffeinate org, so the URL is different. I think everything should redirect just fine, but you may need/want to update your git remote.

@refack
Copy link
Author

refack commented Jan 28, 2017

👍

@refack
Copy link
Author

refack commented Jan 30, 2017

@alangpierce can you review my current work. I got the convert test suite to go green (on my machine). So it's at a good stage.

alangpierce added a commit that referenced this issue Apr 16, 2017
Progress toward #86
Progress toward #98

First step in moving off of nodegit, mostly taken from @refack's branch from #86.
@alangpierce
Copy link
Member

@refack sorry for the delay! nodegit is still causing various issues, so I'm planning to move from it to simple-git based off of your branch. Your branch had a lot of changes, so I'm going to try to break it up into smaller pieces, first focusing on moving off of nodegit and then general Windows support should be a bit more manageable after that.

alangpierce added a commit that referenced this issue Apr 17, 2017
#99)

Progress toward #86
Progress toward #98

First step in moving off of nodegit, mostly taken from @refack's branch from #86.
alangpierce added a commit that referenced this issue Apr 17, 2017
alangpierce added a commit that referenced this issue Apr 17, 2017
alangpierce added a commit that referenced this issue Apr 17, 2017
Fixes #86

This also ended up simplifying the code quite a it, which is nice.
@alangpierce alangpierce assigned alangpierce and unassigned refack Apr 17, 2017
alangpierce added a commit that referenced this issue Apr 17, 2017
Fixes #86

This also ended up simplifying the code quite a it, which is nice.
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

No branches or pull requests

2 participants