[reset_git_repo] only do a full reset when paths is nil #8235

Merged
merged 1 commit into from Feb 14, 2017

Projects

None yet

3 participants

@koenpunt
Contributor
koenpunt commented Feb 14, 2017 edited

Checklist

  • I've run bundle exec rspec from the root directory to see all new and existing tests pass
  • I've followed the fastlane code style and run bundle exec rubocop -a to ensure the code style is valid
  • I've read the Contribution Guidelines
  • I've updated the documentation if necessary.

Description

only do a full reset when paths is nil, so no accidental full-resets can occur when accidentally specifying an empty array

@koenpunt koenpunt only do a full reset when paths is nil
so no accidental full-resets can occur when accidentally specifying an empty array
798f551
@mfurtak
Member
mfurtak commented Feb 14, 2017 edited

While the behavior you're going for seems reasonable to me, the change alters existing behavior that some people may be depending on.

Can you say more about why you think this is important? We should try not to alter existing behavior that people may be using unless absolutely necessary ๐Ÿ‘

@koenpunt
Contributor

I added a glob to the files option, which didn't match any files, and as a result the version change in the xcode project was reverted too..

reset_git_repo files: Dir['ios/**/AppIcon.appiconset/*.png']
@mfurtak
Member
mfurtak commented Feb 14, 2017

Cool, thanks for the example use-case. Your approach was reasonable, and the results were surprising (and destructive) so, let's do it.

@mfurtak mfurtak merged commit 50d2d48 into fastlane:master Feb 14, 2017

1 check passed

ci/circleci Your tests passed on CircleCI!
Details
@mfurtak
Member
mfurtak commented Feb 14, 2017

Thanks for the contribution ๐Ÿ‘

@koenpunt koenpunt deleted the koenpunt:patch-3 branch Feb 14, 2017
@mfurtak mfurtak referenced this pull request Feb 14, 2017
Merged

Version bump to 2.16.0 #8244

@KrauseFx
Member

Great change ๐Ÿ‘

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