search

purpleidea / vagrant-builder

an elegant method for making base images for vagrant-libvirt
GNU Affero General Public License v3.0
42 stars 23 forks source link

Add basic debian support #17

Closed br0ziliy closed 8 years ago

br0ziliy commented 8 years ago

Hey James,

Could you please have a look? I did not test ISO installation with preseed, but the XZ variant is working just fine. What do you think?

purpleidea commented 8 years ago

Hey! This looks pretty awesome. It's almost ready for a merge as-is, but I have some minor review:

1) debian/files/yum.sh # you should probably update this for apt or remove it all together if we're not using it. 2) There are too small issues found by git diff --check: git d 730924d0464b7883e6518600ec34376ddf3b8ca8 3480b576b4a372679c272cfc57f475993a2403e3 --check debian/files/preseed-debian-8.cfg:229: trailing whitespace. +# Next you need to specify the physical partitions that will be used. debian/files/preseed-debian-8.cfg:427: new blank line at EOF.

Other than that, there are a few references to fedora things inside the Makefile, etc but nothing that I have a problem merging!

Thanks :) James

purpleidea commented 8 years ago

btw, mostly curious in case you know, but why is vim called "vim-nox" ?

br0ziliy commented 8 years ago

1) debian/files/yum.sh # you should probably update this for apt or remove it all together if we're not using it.

We're not using it - removed

2) There are too small issues found by git diff --check:

Those are fixed.

Other than that, there are a few references to fedora things inside the Makefile, etc

Did some cleanup of that as well.

btw, mostly curious in case you know, but why is vim called "vim-nox" ?

:) it's because this package "has no X.Org dependencies".

purpleidea commented 8 years ago

Okay thanks merged! Some comments:

1) Next time you should have squashed these two commits into one. Before it's in git master and committed we get to "hide our mistakes" by rebasing. It's good practice too :) Play with git rebase -i in your branch! Never in master!!

2) We're missing a versions/foo.sh script for debian 8 or whatever. Can you send a patch to add this please?

Thanks again for all your work!

br0ziliy commented 8 years ago

Thanks for merging @purpleidea!

1) Next time you should have squashed these two commits into one

Ok, noted.

2) We're missing a versions/foo.sh script for debian 8 or whatever.

That's weird, will look into it. Will do another pull request for that.

purpleidea commented 8 years ago

On Wed, Nov 18, 2015 at 6:00 AM, br0ziliy notifications@github.com wrote:

Ok, noted.

2) We're missing a versions/foo.sh script for debian 8 or whatever.

That's weird, will look into it. Will do another pull request for that.

It's probably because of the .gitignore file (which should be there) but which hid the files you might have added. You'll have to git add -f filename to get past it. This is so that you can some some private versions/*.sh files.