search

projectatomic / vagrant-service-manager

To provide the user a CLI to configure the ADB/CDK for different use cases and to provide glue between ADB/CDK and the user's developer environment.
GNU General Public License v2.0
18 stars 16 forks source link

Fix #348, #357, #365 - install-cli for oc #363

Closed hferentschik closed 8 years ago

hferentschik commented 8 years ago

Follow up on pull request #349 with minor changes and additional update to the README.

Fixes issues #348, #357 and #365 .

hferentschik commented 8 years ago

retest this please

hferentschik commented 8 years ago

@budhrg YEAH, first successful run of service-manager install-cli openshift on CI. And I also ran all tests locally successful and did a manual build, install and test of the gem. Works :-)

hferentschik commented 8 years ago

@budhrg Looking good. Only remaining question I have is in this comment.

@budhrg, @praveenkumar - if you guys have an acceptable answer to why we need to request this specific OpenShift tag, I think we should go ahead and merge and do a 1.2.2 release. Should there be a follow up issue for this test regarding its Vagrant configuration? Is this something we need to re-visit?

@budhrg, do you want to do the release and also verify the release process holds up?

hferentschik commented 8 years ago

For what it's worth:

$ bundle exec rake features CUCUMBER_OPTS='-p all' 
22 scenarios (16 skipped, 6 passed)
450 steps (322 skipped, 128 passed)
15m40.679s

16 min for running all ADB + VirtualBox scenarios. Not too bad and we know we have room for some optimizations. For example the whole refactoring of the help system (so that it works without requiring a running VM) would be a nice user feature, but also help us cutting down time on the Cucumber run time.

navidshaikh commented 8 years ago

Why should we have same commit in two different PRs ? I understand this PR adds the docs for tests. However this can also be handled by adding a collaborator on particular personal namespaced repository (for eg: adding hardy as collaborator on budhram's vsm repo) and then updating the same PR itself. Creating multiple PRs are confusing, reviews and discussions happened on particular topic will be left divided in 2 PRs.

coolbrg commented 8 years ago

@navidshaikh The collaborator way could add more complexity in getting the content if original author want to rebase and work on it again.

Best approach is to create a PR against the author's PR branch for respective changes.

navidshaikh commented 8 years ago

@navidshaikh The collaborator way could add more complexity in getting the content if original author want to rebase and work on it again.

Not really!

Best approach is to create a PR against the author's PR branch for respective changes.

That works too.

The point is to have a same PR and if multiple contributors want to work on it, they should update the same PR, whichever way suites feasible.

hferentschik commented 8 years ago

Why should we have same commit in two different PRs ?

This pull request supersedes the other pull request. I just have not closed the other pull request yet. We have done this now multiple times. Thought I leave it up to you guys. Only one pull request should get applied in the end.

Also, this are NOT the original commits. As you can see, @budhrg pull request failed CI due to some rubocop error and I also adjusted the tag name.

I understand this PR adds the docs for tests. However this can also be handled by adding a collaborator on particular personal namespaced repository (for eg: adding hardy as collaborator on budhram's vsm repo)

That does not make sense. This would imply that everyone has to add everyone to their repos. The way to go is to create your own pull request. Also in some cases the alternative pull request might have a different approach and it is a question which approach gets applied in the end.

Creating multiple PRs are confusing, reviews and discussions happened on particular topic will be left divided in 2 PRs.

I don't see the confusion here. I probably should have added a comment directly to the original pull request and maybe even already closed it. My fault I guess.

hferentschik commented 8 years ago

Best approach is to create a PR against the author's PR branch for respective changes.

That's a possibility a well. In this case the author needs to merge first into his fork and then update his pull request. A bit more work really. It depends on a case to case basis.

hferentschik commented 8 years ago

I closed @budhrg original pull request, so it is only this one which stands now. I hope this resolves the confusion.

coolbrg commented 8 years ago

In this case the author needs to merge first into his fork and then update his pull request.

@hferentschik , just to bring here, its just first merge into his fork and PR will automatically get updated as base base code(fork) is updated.

coolbrg commented 8 years ago

do you want to do the release and also verify the release process holds up?

@hferentschik Sure. :+1:

coolbrg commented 8 years ago

retest this please