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

[precheck] Allow mentioning Google Drive #13754

Merged
merged 2 commits into from
Nov 24, 2018

Conversation

douglashill
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

This fixes a false positive in precheck. It is reasonable that apps might integrate with Google Drive and users would want to know about this from the App Store description. Our app PDF Viewer has been approving mentioning Google Drive.

Description

I added an additional entry to the whitelist for mentioning other platforms. Google is on the blacklist because it is a competitor to Apple, then Google Analytics and Google Drive are whitelisted because iOS app may integrate with these services.

I added tests for Precheck::OtherPlatformsRule, just covering a one allowed case and one not allowed case. I don’t really know Ruby or any of the rig here so I copied and modified curse_words_rule_spec.rb.

The allowed case fails because Google is flagged.
Our app PDF Viewer has been approved while mentioning Google Drive in the App Store description. Other apps that use UIDocumentBrowserViewController may want to mention this too.

The OtherPlatformsRule tests now pass.
@googlebot

This comment has been minimized.

@douglashill

This comment has been minimized.

@googlebot
Copy link

CLAs look good, thanks!

@janpio
Copy link
Member

janpio commented Nov 23, 2018

Thanks for these changes!

Semi OT: Is mentioning "Google" actually a problem? "Android" I get, but the company in general?

@douglashill
Copy link
Contributor Author

douglashill commented Nov 23, 2018

I don’t know. I'm sceptical about whether most of these other platform checks are useful. Most of them seem irrelevant these days and unlikely to be used or be a problem. Android seems a good warning as that's definitely caused some people trouble in the past. Google seems a bad warning to me. We already have two examples, and there are lots more Google services an iOS might mention reasonably. Or the app might be one made by Google itself :)

So if it might be accepted, I'd say +1 to just removing google from the blacklist.

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.

Perfect. I don't think Apple actually specified the exact rules around Google related products, so we can only add rules based on experiences by the community 👍 Let's go with this

describe Precheck::OtherPlatformsRule do
let(:rule) { OtherPlatformsRule.new }
let(:allowed_item) { TextItemToCheck.new("We have integration with the Files app so you can open documents stored in your Google Drive.", :description, "description") }
let(:forbidden_item) { TextItemToCheck.new("Google is really great.", :description, "description") }
Copy link
Member

Choose a reason for hiding this comment

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

💯

@KrauseFx KrauseFx merged commit ccfcd4e into fastlane:master Nov 24, 2018
@fastlane-bot
Copy link

Hey @douglashill 👋

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 🚀

@douglashill douglashill deleted the precheck-google-drive branch November 26, 2018 02:22
mariannegru added a commit to mariannegru/fastlane that referenced this pull request Dec 3, 2018
…select

* upstream/master: (34 commits)
  Use HTTPs for Facebook frame URL (fastlane#13771)
  [spaceship] improve client (mainly: login) (fastlane#13736)
  [spaceship] Fix environment variable provided sessions (fastlane#13743)
  Allow for the host of the Appetize API to be configured (fastlane#13765)
  Improves parse error reporting for non-conventional setups (fastlane#13769)
  Use `crash!` instead of `user_error!` on configuration access mis-use (fastlane#13766)
  Rip out more dead analytics code (fastlane#13741)
  [spaceship] support --verbose for `fastlane spaceauth` (fastlane#13752)
  [fastlane_core] Fix project_paths() in project.rb to respect `container:` references (fastlane#13662)
  [precheck] Allow mentioning Google Drive (fastlane#13754)
  [crashlytics] fix generated command (generate_android_command) for Windows (fastlane#13597)
  [docs] add id to plugin headline so it becomes linkable (fastlane#13727)
  [match] add new option to recreate deleted profiles in dev portal (fastlane#12539)
  Update puts.rb (fastlane#13739)
  Improve Spaceship API documentation (fastlane#13724)
  Update gym docs (fastlane#13725)
  Testfairy upload timeout (fastlane#13674)
  [resign.sh] only create the archived-expanded-entitlements.xcent file if the version of Xcode < 9.3 (fastlane#13685)
  [spaceship] remove application and version from app submission params (fastlane#13695)
  Improve AdbHelper (fastlane#13692)
  ...

# Conflicts:
#	spaceship/lib/spaceship/two_step_or_factor_client.rb
@fastlane-bot
Copy link

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

@fastlane fastlane locked and limited conversation to collaborators Feb 4, 2019
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.

5 participants