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

don't check git connection #1216

Closed

Conversation

teaforthecat
Copy link

It may not be required or allowed to have a connection to the git
server from the app host. When using capistrano-rsync it isn't even required to have git
installed. I don't see why this is required. Perhaps the check can be
moved to just before the create_release command, but I don't see the
point in doing that.

We are using capistrano-rsync for the purpose of not having a network connection from the app server to the office git server, and it is just a bonus that git isn't even required to be installed. Hopefully the problem that the check is solving can be solved in another location or can be opt-in some how. I don't see a reason for it though, so I just removed it.

It may not be required or allowed to have a connection to the git
server from the app host. When using capistrano-rsync it isn't even required to have git
installed. I don't see why this is required. Perhaps the check can be
moved to just before the create_release command, but I don't see the
point in doing that.
@leehambley
Copy link
Member

That will break deploys for people who do rely on that, I am sure. We can't take a change like this without corresponding feature specs.

@teaforthecat
Copy link
Author

I don't see how it would break deploys. It would only move the error down the execution path a little. I understand you're hesitation though. Hopefully there is some solution to allow the use of capistrano in my situation. I tried overriding the task and the variables, to no avail. Do you see a way to prevent the "git ls-remote" command from happening? Thanks

@leehambley
Copy link
Member

I'm sure it's a good idea, but we'd have to address something as fundamental as this with higher level cucumber specs, maybe you can take a look at how they run now rake features and simply ammend your PR?

@Kriechi
Copy link
Contributor

Kriechi commented Jan 16, 2015

Why can't capistrano-rsync just implement a NullObject pattern for the scm tasks?
As far as I understand the current code is already doing something similar. It should just overwrite or implement the scm-tasks properly.

@teaforthecat
Copy link
Author

Upon further review, I wasn't using capistrano-rsync correctly. Thanks @Kriechi for pointing me in the right direction. I have a pull request for capistrano-rysc here moll/capistrano-rsync#20

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants