Add path option to delete_keychain action #8003

Open
wants to merge 3 commits into
from

Projects

None yet

4 participants

@koenpunt
Contributor
koenpunt commented Jan 25, 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

Add path option to specify a keychain somewhere else than ~/Library/Keychains

Motivation and Context

This complements the create_keychain command, which already supports specifying the keychain with a path.

@asfalcone
Member

@koenpunt thanks for the PR. It looks like tests are failing. Please run the tests locally to fix.

Also, please complete the checklist in the PR as well as fill out the Description and Context sections of the template. Thanks!

@koenpunt
Contributor

I left the checklist open for a reason :) But now it's done

@koenpunt koenpunt referenced this pull request Jan 26, 2017
Open

Add keychain_path option to import_certificate action #8009

4 of 4 tasks complete
@koenpunt koenpunt changed the title from Always provide keychain as absolute path to Add path option to delete_keychain action Jan 26, 2017
@KrauseFx
Member

I left the checklist open for a reason :) But now it's done

Cool, no worries, usually we mark PRs that aren't ready to be merged yet as [WIP] ๐Ÿ‘ Thanks for updating the PR

- optional: false)
+ conflicting_options: [:path],
+ optional: true),
+ FastlaneCore::ConfigItem.new(key: :path,
@KrauseFx
KrauseFx Jan 26, 2017 Member

I know some other actions are using this name already, but I'd prefer the name to be keychain_path to be more explicit ๐Ÿ‘

@koenpunt
koenpunt Jan 26, 2017 edited Contributor

Should we change this for create_keychain too then?

@KrauseFx
KrauseFx Jan 26, 2017 Member

We can't change option names, as this would break setups, let's leave it like it is for now ๐Ÿ‘

@KrauseFx
KrauseFx Jan 26, 2017 Member

Meaning we should change it here, but leave it for create_keychain

@koenpunt
koenpunt Feb 15, 2017 Contributor

Done

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