Make SourcesManager ignore some global git configs #1978

Merged
merged 3 commits into from Mar 30, 2014

3 participants

@neonichu
CocoaPods member

As discussed in #1949

@fabiopelosin
CocoaPods member

Looks good to me, but I would prefer the test to be separated... something:

it "is robust against user settings for git repos" do
  command = [...]
  command.should.include?('--no-rebase')
end

it "doesn't allow merge commits" do
  command = [...]
  command.should.include?('--no-commit')
end
@neonichu
CocoaPods member

Seems like a lot of redundancy in the specs for little gain. The options will always be included regardless of the actual config, anyways.

@fabiopelosin
CocoaPods member

I prefer to have single specs methods as this makes more unlikely and accidental remove of the feature. I'm also of the school of though that one test should check only one thing. Finally the specs in this case serve also for documentation as it can be unclear in future why the flags where added.

@neonichu
CocoaPods member

OK, makes sense. Added two new specs, not sure if this is what you had in mind.

@fabiopelosin
CocoaPods member

Sweeeeet... thanks for cooping with my pedanticness 🍻

@fabiopelosin
CocoaPods member

Merging because the CI error is unrelated

@neonichu
CocoaPods member

It's your show ;) 🍻

@fabiopelosin fabiopelosin merged commit 027b9f2 into CocoaPods:master Mar 30, 2014

1 check failed

Details default The Travis CI build failed
@neonichu neonichu added a commit to neonichu/CocoaPods that referenced this pull request Mar 30, 2014
@neonichu neonichu Updated CHANGELOG for #1978 98df1a7
@neonichu neonichu added a commit to neonichu/CocoaPods that referenced this pull request Mar 30, 2014
@neonichu neonichu Updated CHANGELOG for #1978 0eae7ad
@ragnar

This breaks pod repo update for me. It won't commit fetched commit and it will fail with the following error without going into ~/.cocoapods/repo/master and reseting the repo first.

[!] Pod::Executable pull --no-rebase --no-commit

You have not concluded your merge (MERGE_HEAD exists).

Please, commit your changes before you can merge.

-r-

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