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

1612 handle concurrency of rugged and cli git #1677

Open
wants to merge 5 commits into
base: staging
Choose a base branch
from

Conversation

phyrog
Copy link
Contributor

@phyrog phyrog commented May 9, 2016

This should fix #1612

end

# change a single file and commit the change
def commit_file_without_lock(userinfo, file_contents, target_path, message, &block)

Choose a reason for hiding this comment

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

Line is too long. [85/80]
Method has too many lines. [22/10]

@phyrog
Copy link
Contributor Author

phyrog commented May 9, 2016

I'll ignore the Method has too many lines messages since they refer to methods that were not written in this PR and should be addressed in a refactoring.

end

# change a single file and commit the change
def commit_file_without_lock(userinfo,

Choose a reason for hiding this comment

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

Method has too many lines. [26/10]

@@ -26,6 +26,12 @@ def initialize(repo_path, key_id, refs)
end

def exec
Semaphore.exclusively("action:#{@repo_name}") do
Copy link
Member

Choose a reason for hiding this comment

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

Please find a way to only write "action:" once. Maybe put it into a constant or a class method of GitRepository.
And please use git instead of action (we will have many locks with different types and all of them are some kind of action)

@phyrog phyrog force-pushed the 1612-handle_concurrency_of_rugged_and_cli_git branch from 55e3375 to b33e751 Compare May 31, 2016 07:26
@eugenk
Copy link
Member

eugenk commented Jun 6, 2016

I think you could incorporate it as well in lib/git_repository/cloning.rb:98 (git_exec) just to be sure.

A mirror repository should not be synchronised while another operation is executed on the repository, but a lock shouldn't hurt in this case.

@eugenk
Copy link
Member

eugenk commented Jul 19, 2016

See spec/lib/semaphore_spec.rb for a test that uses fork_break

@phyrog phyrog force-pushed the 1612-handle_concurrency_of_rugged_and_cli_git branch from b33e751 to 2e4d514 Compare August 8, 2016 11:02
end

# change a single file and commit the change
def commit_file_without_lock(userinfo,

Choose a reason for hiding this comment

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

Method has too many lines. [26/10]

Copy link
Member

Choose a reason for hiding this comment

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

This is not related to the PR, so don't mind.

@phyrog phyrog force-pushed the 1612-handle_concurrency_of_rugged_and_cli_git branch from 2e4d514 to bda5480 Compare August 8, 2016 11:20
@eugenk
Copy link
Member

eugenk commented Aug 10, 2016

Well, except for the yet to be written tests: 👍

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.

Handle concurrency of rugged and CLI git
3 participants