Skip to content
This repository has been archived by the owner on Jan 18, 2023. It is now read-only.

using ls-remote instead of rev-parse to fetch revision #19

Merged
merged 2 commits into from
Nov 3, 2015
Merged

using ls-remote instead of rev-parse to fetch revision #19

merged 2 commits into from
Nov 3, 2015

Conversation

ricoli
Copy link
Contributor

@ricoli ricoli commented Oct 28, 2015

As per #18

def revision
@revision ||= `git rev-parse #{branch}`.chomp
@revision ||= `git ls-remote #{repository} #{branch}`.chomp
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I run this locally, I get soemthing like:

aa04140db6643ac65152cba813085815dfeb1141    refs/heads/master

It looks like this will need more modification! .split(" ").first

@@ -91,8 +91,12 @@ def destination
fetch(:slack_destination, stage)
end

def repository
fetch(:repository, 'origin')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

origin is a remote, not a repository. what's your reasoning for using :repository here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's my understanding (might be completely wrong) that :repository would be the capistrano's config for the remote's url?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's true! just making sure we clarified that :)

parkr added a commit that referenced this pull request Nov 3, 2015
@parkr parkr merged commit e6754c3 into parkr:master Nov 3, 2015
parkr added a commit that referenced this pull request Nov 3, 2015
@ricoli ricoli deleted the revision-from-remote branch November 11, 2015 22:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants