minishift / minishift-centos-iso

CentOS based ISO as an alternative for boot2docker ISO
GNU Lesser General Public License v3.0
40 stars 33 forks source link

Issue #96 Move swapon to mount function #97

Closed gbraad closed 7 years ago

gbraad commented 7 years ago

Addresses issue #96

centos-ci commented 7 years ago

Can one of the admins verify this patch?

hferentschik commented 7 years ago

add to whitelist

coolbrg commented 7 years ago

@gbraad Could you use this pattern for you commit message? We are kind of following it throughout our projects.

Issue #<issue_id> message

Eg: In your case, it would be "Issue #96 Move swapon to mount function"

Thanks, Just want to make sure we are consistent across the board.

gbraad commented 7 years ago

@budhrg changing...

Pushed a new commit

hferentschik commented 7 years ago

Could you use this pattern for you commit message? We are kind of following it throughout our projects.

+1

@gbraad thanks for updating the pull request :-)

hferentschik commented 7 years ago

@gbraad Looking good. Should we add a test? We could extend on the existing tests. Do something like:

The rub atm is unfortunately, the tests currently will only work on Linux. There are several things which need addressing to get these tests to run on any OS. @budhrg, did we not have a follow up issue which should make the test harness work cross platform?

Either way, it would be fine for me to add tests and basically just verify that they pass on CI.

gbraad commented 7 years ago

@hferentschik How would we run the tests? these are independent of the minishift integration tests and should be considered smoke tests... is this advocado? But yes, the suggested scenario seems reasonable.

hferentschik commented 7 years ago

How would we run the tests?

Not sure what you mean? I guess you are wondering how to start the VM? Well, you have two choices, either you just start them via minishift. Nothing wrong with that really. It does more than we want, but for what we are after it works.

The alternative is to use docker-machine which would be more dedicated to what we want. In fact there used to be docker-machine smoke tests as well, since we wanted to make sure that the ISO keeps working for pure docker-machine as well. Not sure what happened to these tests. @budhrg do you still have these? Did we not want to add them to make sure we keep compatibility for docker-machine?

@gbraad Provided we can dig these docker-machine stuff, one could use that to control the VM.

these are independent of the minishift integration tests and should be considered smoke tests

sure, I also consider these minishift tests as smoke tests. We could just add another test file to separate the tests better, but yes the easiest would be to use minishift to stop/start the VM.

gbraad commented 7 years ago

@hferentschik I have actually found the answer to my earlier question. The tests are very simplistic in setup... but yes, I am also interested in the the docker-machine-based tests

Off-topic rant: why do we write "Issue #" as it actually fixes or address an issue, I have also noticed "Fix #" which gramatically makes more sense. Anyways, I believe the important part is #nnn to refer to an issue. ;-)

hferentschik commented 7 years ago

Merged, thanks

hferentschik commented 7 years ago

Merged, thanks