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

Require version file for checking capistrano version #2616

Merged
merged 3 commits into from Sep 1, 2013

Conversation

Projects
None yet
3 participants
@sanemat
Contributor

sanemat commented Aug 30, 2013

bundler-1.4.0.pre.1 works fine, but bundler-1.4.0.pre.2 shows error below:

$ bundle exec cap production deploy
/path/to/bundler-1.4.0.pre.2/lib/bundler/capistrano.rb:7:in `<top (required)>': uninitialized constant Capistrano::Version (NameError)

This commit brings this error, so I add require version file.
1d7a789

bundler: 1.4.0.pre.2
capistrano: 2.14.2

Require version file for checking capistrano version
bundler-1.4.0.pre.1 works fine, but bundler-1.4.0.pre.2 shows error below:
```
$ bundle exec cap production deploy
/path/to/bundler-1.4.0.pre.2/lib/bundler/capistrano.rb:7:in `<top (required)>': uninitialized constant Capistrano::Version (NameError)
```

This commit brings this error, so I add require version file.
1d7a789

bundler: 1.4.0.pre.2
capistrano: 2.14.2
@sanemat

This comment has been minimized.

Show comment
Hide comment
@sanemat
Contributor

sanemat commented Aug 30, 2013

@kirs

This comment has been minimized.

Show comment
Hide comment
@kirs

kirs Aug 30, 2013

Contributor

Sorry, my bad :(

Contributor

kirs commented Aug 30, 2013

Sorry, my bad :(

@indirect

This comment has been minimized.

Show comment
Hide comment
@indirect

indirect Aug 30, 2013

Member

Do all versions of capistrano supply capistrano/version? Or do we need to rescue the potential LoadError?

Member

indirect commented Aug 30, 2013

Do all versions of capistrano supply capistrano/version? Or do we need to rescue the potential LoadError?

@sanemat

This comment has been minimized.

Show comment
Hide comment
@sanemat

sanemat Aug 30, 2013

Contributor

@indirect
Ignore LoadError and check defined? const Capistrano::Version.

Future:
capistrano v3(at least 3.0.0.pre14)
capistrano/version exists, but I don't know this exists as ever.
(require 'capistrano' requires capistrano/version)

Past:
capistrano v1.3.0(2006-12-24) already had capistrano/version.
(require 'capistrano' does not require capistrano/version)

Contributor

sanemat commented Aug 30, 2013

@indirect
Ignore LoadError and check defined? const Capistrano::Version.

Future:
capistrano v3(at least 3.0.0.pre14)
capistrano/version exists, but I don't know this exists as ever.
(require 'capistrano' requires capistrano/version)

Past:
capistrano v1.3.0(2006-12-24) already had capistrano/version.
(require 'capistrano' does not require capistrano/version)

@indirect

This comment has been minimized.

Show comment
Hide comment
@indirect

indirect Aug 30, 2013

Member

Sounds like we can simply check defined?(Capsitrano::VERSION) instead of requiring it ourselves, then, since we only care about the version if it's required automatically already.

Member

indirect commented Aug 30, 2013

Sounds like we can simply check defined?(Capsitrano::VERSION) instead of requiring it ourselves, then, since we only care about the version if it's required automatically already.

@sanemat

This comment has been minimized.

Show comment
Hide comment
@sanemat

sanemat Aug 30, 2013

Contributor

I agree 😃

Contributor

sanemat commented Aug 30, 2013

I agree 😃

@sanemat

This comment has been minimized.

Show comment
Hide comment
@sanemat

sanemat Aug 30, 2013

Contributor

oops, I don't know capistrano v3 and later require 'capistrano requires capistrano/version as ever.

Contributor

sanemat commented Aug 30, 2013

oops, I don't know capistrano v3 and later require 'capistrano requires capistrano/version as ever.

@sanemat

This comment has been minimized.

Show comment
Hide comment
@sanemat

sanemat Aug 30, 2013

Contributor

It is not problem, so I simply check Capistrano::Version.

Contributor

sanemat commented Aug 30, 2013

It is not problem, so I simply check Capistrano::Version.

@sanemat

This comment has been minimized.

Show comment
Hide comment
@sanemat

sanemat Sep 1, 2013

Contributor

@indirect Would you merge this?

Contributor

sanemat commented Sep 1, 2013

@indirect Would you merge this?

@indirect

This comment has been minimized.

Show comment
Hide comment
@indirect

indirect Sep 1, 2013

Member

I was waiting for the specs to be green, but I see it was a random failure. Thanks for the patch!

Member

indirect commented Sep 1, 2013

I was waiting for the specs to be green, but I see it was a random failure. Thanks for the patch!

indirect added a commit that referenced this pull request Sep 1, 2013

Merge pull request #2616 from sanemat/fix/if-capistrano
Require version file for checking capistrano version

@indirect indirect merged commit 0ab0cd9 into bundler:master Sep 1, 2013

1 check was pending

default The Travis CI build is in progress
Details
@sanemat

This comment has been minimized.

Show comment
Hide comment
@sanemat

sanemat Sep 1, 2013

Contributor

Thanks! 😍😍

Contributor

sanemat commented Sep 1, 2013

Thanks! 😍😍

@sanemat sanemat deleted the sanemat:fix/if-capistrano branch Sep 1, 2013

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