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

Development: Add default values to all arguments in add_certificates and use_profiles declarations #226

Merged

Conversation

DetachHead
Copy link
Contributor

@DetachHead DetachHead commented May 15, 2022

this makes the api behave more like the cli and makes it much easier to use.

currently in my code i have to do this, which isn't ideal and adds unnecessary bloat to my script:

XcodeProject().use_profiles(
    XcodeProjectArgument.XCODE_PROJECT_PATTERN.get_default(),
    XcodeProjectArgument.PROFILE_PATHS.get_default(),
)

@DetachHead DetachHead changed the title use default value for certificate_path_patterns parameter in Keychain.add_certificates add missing default values May 15, 2022
Copy link
Contributor

@priitlatt priitlatt left a comment

Choose a reason for hiding this comment

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

As these changes are not breaking and they make the usage of Python API a bit more convenient, then in principle I don't have anything to object. But please could you update changelog too with the changes you've made. Version header could be Unreleased and the notes should go under development section.

@priitlatt priitlatt added the enhancement New feature or request label May 17, 2022
@priitlatt priitlatt changed the title add missing default values Development: Add default values to all arguments in add_certificates and use_profiles declarations May 17, 2022
@priitlatt
Copy link
Contributor

Please add PR description that outlies changes and the rationale for what was done.

@DetachHead DetachHead force-pushed the Keychain.add_certificates_default branch from 3da1012 to 218b78c Compare May 18, 2022 08:10
@DetachHead DetachHead requested a review from priitlatt May 18, 2022 08:27
@DetachHead DetachHead force-pushed the Keychain.add_certificates_default branch from 218b78c to 009479b Compare May 26, 2022 10:01
@DetachHead DetachHead requested a review from priitlatt May 26, 2022 10:03
Copy link
Contributor

@priitlatt priitlatt left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you for your contribution!

@priitlatt priitlatt merged commit 2a80e6b into codemagic-ci-cd:master May 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants