[go: nahoru, domu]

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][pod_lib_lint] add "configuration" option #19582

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

Conversation

siejkowski
Copy link

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

Implements #19581

Description

Added "configuration" option to the pod_lib_lint action. This maps to the "--configuration" option that is available in CocoaPods since v1.10.

Testing Steps

Apart from added unit tests, I've tested the setup locally with a pod I maintain and the lane like:

  lane :test_fastlane_change do
    pod_lib_lint(
      podspec: "POD_NAME.podspec",
      allow_warnings: true,
      skip_tests: true,
      platforms: "macos",
      no_subspecs: true,
      configuration: 'Debug'
    )
  end

@google-cla google-cla bot added the cla: no label Nov 5, 2021
@siejkowski
Copy link
Author

I've just submitted the Google CLA

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.

One small question on if a configuration has a space in the name! Otherwise this looks good 💪

@@ -27,6 +27,7 @@ def self.run(params)
command << "--skip-import-validation" if params[:skip_import_validation]
command << "--skip-tests" if params[:skip_tests]
command << "--analyze" if params[:analyze]
command << "--configuration=#{params[:configuration]}" if params[:configuration]
Copy link
Member

Choose a reason for hiding this comment

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

What happens if a configuration has a space in the name? 🤔

I'm thinking maybe we need to either wrap this in quotes or call .shellescape on it? Thoughts?

@Kukurijek
Copy link
Contributor

Hi @siejkowski !

Could you rebase the master and add the CLA ? 😄

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

5 participants