Skip to content

Feature: Add Ability to create recipe with specific commits#117

Open
larebsyed wants to merge 7 commits intorejeep:masterfrom
larebsyed:master
Open

Feature: Add Ability to create recipe with specific commits#117
larebsyed wants to merge 7 commits intorejeep:masterfrom
larebsyed:master

Conversation

@larebsyed
Copy link

Usecase:
You need to test different committed version of same branch because sometime, a bug is introduced in the latest snapshot branch and we have to track back to commit which was source of the bug.

git_repo = Evm::Git.new(build_path)
if git_repo.exist?
git_repo.pull
if git_repo.exist?
Copy link
Collaborator

Choose a reason for hiding this comment

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

Indentation appears to have been broken here.

def clone(url, branch = nil, commit = nil)
args = 'clone', url, @path
args << "--branch=#{branch}" if branch
if commit == nil
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think nil? is more Ruby-ish? Maybe cleaner to pass this in as a boolean instead of the (unused) actual commit?

Copy link
Collaborator

@sambrightman sambrightman left a comment

Choose a reason for hiding this comment

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

Like the idea, feels like implementation could be cleaner. I've only glanced over it though, let me know what I've misunderstood.

Dir.chdir(@path) do
git 'pull'
args = 'pull'
args = 'pull', '--depth=2000000' if commit
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is pretty hacky. Is it really the only way to unshallow the repository? I have a feeling --unshallow --tags works, but haven't tested. The change is not fully described in the commit message, so hard to tell.

end
end

def reset(commit = nil)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why reset instead of checkout?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants