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

[match] Adds identifiers supplied to match to the git commit message #19303

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mikelrob
Copy link
Contributor

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.

Motivation and Context

We have multiple teams developing multiple apps. We all use fastlane to automate our deployment, it's so great to have a reliable automated process. Recently we had to renew our iOS Distribution certificate which resulted in all teams renewing their respective Distribution provisioning profiles. This was super easy using fastlane match however the git repo we use for storage does not have a readable commit log. I wanted to include the identifiers supplied to match from the CLI or Matchfile in the commit message to give us better visibility at a glance of our log.

Description

First I moved the app_identifiers block of code in the runner above the storage selection. I want to pass this into the storage object.
I fixed up the runner_spec according to expect these extra params.
Next I added the identifiers to the git_storage and used them in the generate_commit_message func.
I updated the git_storage_spec with context block testing for single and multiple identifiers.
Ensuring it works with multiple identifers was important to us as we have an app with linked extensions.
After the changes were made I ran the test suite and rubocop analysis.

Testing Steps

I ran:

bundle exec rspec
bundle exec rubocop -a

Adds spec context for single and multiple identifier
@google-cla google-cla bot added the cla: yes label Aug 27, 2021
Copy link
Member

@joshdholtz joshdholtz left a comment

Choose a reason for hiding this comment

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

I think this is a great addition! However, I think this could be reworked to use the existing custom_message argument on def save_changes!(files_to_commit: nil, files_to_delete: nil, custom_message: nil). This way you don't need to pass the identifiers into the storage object 😇

Let me know your thoughts on this!

@mikelrob
Copy link
Contributor Author

mikelrob commented Sep 2, 2021

I think this is a great addition! However, I think this could be reworked to use the existing custom_message argument on def save_changes!(files_to_commit: nil, files_to_delete: nil, custom_message: nil). This way you don't need to pass the identifiers into the storage object 😇

Let me know your thoughts on this!

Thanks! Yea, I like your idea of using the existing mechanism. I guess I just need to create an option on the match command to accept the custom message and pass it through?

@joshdholtz
Copy link
Member

I think this is a great addition! However, I think this could be reworked to use the existing custom_message argument on def save_changes!(files_to_commit: nil, files_to_delete: nil, custom_message: nil). This way you don't need to pass the identifiers into the storage object 😇
Let me know your thoughts on this!

Thanks! Yea, I like your idea of using the existing mechanism. I guess I just need to create an option on the match command to accept the custom message and pass it through?

I think adding a new option for that would make sense! If that is something you think you'd be able to use how you want 😇 With the option somebody could use it for app identifiers or maybe the person's name who ran it 🤷‍♂️

@CaseyCw1

This comment was marked as spam.

@getaaron
Copy link
Collaborator

@mikelrob hey are you still working on this? we'd love to get it fixed up and merged in 😍

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

Successfully merging this pull request may close these issues.

None yet

6 participants