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

Add git_user_name and git_user_email option to match #8550

Merged
merged 3 commits into from
Mar 17, 2017
Merged

Add git_user_name and git_user_email option to match #8550

merged 3 commits into from
Mar 17, 2017

Conversation

kouki-dan
Copy link
Contributor

@kouki-dan kouki-dan commented Mar 16, 2017

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 new option to match git_user_name and git_user_email to set git config to match's git repository.

Motivation and Context

If using useConfigOnly option (and global name or email settings is not set) in gitconfig, match fails to commit to git repository
Add new two option for name and email and fix it.

The reason which the git option needs is followed by this page

If using useConfigOnly option (and global name or email settings is not set) in gitconfig, match fails to commit to git repository
Add new two option for name and email and fix it.
@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@kouki-dan
Copy link
Contributor Author

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

@@ -64,6 +64,16 @@ def self.available_options
verify_block: proc do |value|
ENV["FASTLANE_TEAM_ID"] = value.to_s
end),
FastlaneCore::ConfigItem.new(key: :git_user_name,
env_name: "MATCH_GIT_USER_NAME",
description: "git user name to commit",
Copy link
Member

Choose a reason for hiding this comment

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

Maybe that's just a small detail, but by user name many people understand the nickname (in my case KrauseFx), however in git it's recommended to use the full name as far as I know (in my case Felix Krause). Maybe we should rename the key, env_name and description to clarify this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your review. I understand.
I will replace user name to full name and push this.

@KrauseFx
Copy link
Member

Hey @kouki-dan, thanks for taking the time to submit this PR, a great change, and I can totally see the use case for this 👍

commands << "git config user.name \"#{user_name}\"" unless user_name.nil?
commands << "git config user.email \"#{user_email}\"" unless user_email.nil?

UI.message "Add git user config to local git repo..." unless commands.empty?
Copy link
Member

Choose a reason for hiding this comment

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

Maybe you can prepend this with a line that's just

return if commands.empty?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, okay. That's better I think.

Copy link
Member

@KrauseFx KrauseFx left a comment

Choose a reason for hiding this comment

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

This looks great 🎷

@KrauseFx KrauseFx merged commit 91980e5 into fastlane:master Mar 17, 2017
@KrauseFx
Copy link
Member

Thanks for your contribution @kouki-dan 👍

@hjanuschka
Copy link
Collaborator

i think that fixes: #8450

@kouki-dan
Copy link
Contributor Author

Yes, I got a same error as #8450.

Thank you for your reviewing and merging. I support Fastlane project 👍

@kouki-dan kouki-dan deleted the git-commit-with-useConfigOnly-option-of-git branch March 17, 2017 06:55
@KrauseFx KrauseFx mentioned this pull request Mar 20, 2017
@fastlane-bot
Copy link

Congratulations! 🎉 This was released as part of fastlane 2.21.0 🚀

@ky1ejs
Copy link
Contributor

ky1ejs commented May 3, 2017

This is an awesome improvement! This problem got me for ages. I had to keep changing my gitconfig back and forth which was error prone. Thank you!

However, I'm not sure why, but this isn't working for me. I'm running fastlane 2.28.7.

Tried:

$ fastlane match -a my.app.id --git_full_name 'Fastlane Match' --git_user_email 'match@fastlane.tools'

and

# Fastfile
match(app_identifier: identifier,
          readonly: readonly,
          type: type,
          shallow_clone: 1,
          git_full_name: 'Fastlane Match',
          git_user_email: 'match@fastlane.tools')

Any idea as to what I'm doing wrong?

@KrauseFx
Copy link
Member

KrauseFx commented May 3, 2017

@kylejm that's how you should use it. Can you submit a new issue with the output? Is it not working?

@ky1ejs
Copy link
Contributor

ky1ejs commented May 7, 2017

Hey @KrauseFx, sorry for the slow response. Issue created ☝️.

@fastlane fastlane locked and limited conversation to collaborators Aug 6, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants