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

[action][git_commit] remove all instances of is_string in options and use type #18883

Merged
merged 4 commits into from Jun 15, 2021

Conversation

crazymanish
Copy link
Member

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

  • is_string is slowly being replaced by type to make the docs clearer, options safer, and Swift generation more correct.
  • This PR does this for only git_commit action.

Description

  • Remove is_string: true from options

Testing Steps

  • bundle exec rspec fastlane/spec/actions_specs/git_commit_spec.rb
  • No functionality changed, all existing/new unit tests should pass.

@google-cla google-cla bot added the cla: yes label Jun 13, 2021
@@ -71,7 +67,7 @@ def self.is_supported?(platform)

def self.example_code
[
'git_commit(path: "./version.txt", message: "Version Bump")',
# 'git_commit(path: "./version.txt", message: "Version Bump")',
Copy link
Member Author

Choose a reason for hiding this comment

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

Force to disable this otherwise test_sample_code is failing...same as last PR: #18882 (comment)
waiting for Josh's recommendation here 😇

Screenshot 2021-06-13 at 16 52 12

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

@crazymanish What did you run to get this error? 🤔 I checked out this branch and wasn't getting this 😛

Copy link
Member Author

Choose a reason for hiding this comment

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

Step1: Uncomment below example code (remove #)
# 'git_commit(path: "./version.txt", message: "Version Bump")',

Step2: Run bundle exec fastlane validate_docs

Copy link
Member

Choose a reason for hiding this comment

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

Oh, duh! You commented them out 😛

@google-cla
Copy link

google-cla bot commented Jun 15, 2021

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added cla: no and removed cla: yes labels Jun 15, 2021
@joshdholtz
Copy link
Member

@googlebot I consent.

@google-cla google-cla bot added cla: yes and removed cla: no labels Jun 15, 2021
@joshdholtz
Copy link
Member

@crazymanish Added a fix for test_sample_code to call a new method called ensure_array_type_passes_validation 😊

@crazymanish
Copy link
Member Author

@crazymanish Added a fix for test_sample_code to call a new method called ensure_array_type_passes_validation 😊

Thank you, thank you! 🚀
once you merge this into master, I will rebase other PR#18882 also and make it ready for your review! 💪


# Arrays can be an either be an array or string that gets split by comma in auto_convert_type
if !value.kind_of?(Array) && !value.kind_of?(String)
UI.user_error!("'#{self.key}' value must be either `true` or `false`! Found #{value.class} instead.")
Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
UI.user_error!("'#{self.key}' value must be either `true` or `false`! Found #{value.class} instead.")
UI.user_error!("'#{self.key}' value must be either `array` or `comma seperated string`! Found #{value.class} instead.")

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.

🔥

@joshdholtz joshdholtz merged commit ec7625e into master Jun 15, 2021
@joshdholtz joshdholtz deleted the crazymanish-git_commit-clean-up branch June 15, 2021 19:30
@fastlane-bot
Copy link

Hey @crazymanish 👋

Thank you for your contribution to fastlane and congrats on getting this pull request merged 🎉
The code change now lives in the master branch, however it wasn't released to RubyGems yet.
We usually ship about once a week, and your PR will be included in the next one.

Please let us know if this change requires an immediate release by adding a comment here 👍
We'll notify you once we shipped a new release with your changes 🚀

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

3 participants