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

[frameit] Automatically determine platform from fastfile #16428

Merged
merged 29 commits into from
May 12, 2020

Conversation

milhauscz
Copy link
Contributor

@milhauscz milhauscz commented May 6, 2020

🔑

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

In my previous PR #15087 I added the support for Android to frameit. A part of the mechanism for detecting the platform whose frames should be added to screenshots is new "use_platform" configuration and new commands. Currently frameit determines the platform as follows (sorted according to priority):

  1. "use_platform" key in Framefile.json
  2. command entered directly to CLI: fastlane frameit android or fastlane frameit ios
  3. "use_platform" parameter in fastfile's lane or entered to CLI where default is IOS for backward compatibility, e. g. fastlane run frameit use_platform:"ANDROID"

If the user has a fastfile with the frameit lane, it is still necessary to provide the platform configuration via one of the described options, otherwise frameit falls back to "IOS" for backward compatibility.

Description

This PR changes this behavior: if fastfile is used and no other configuration provided, the platform is determined based on fastfile's default platform configuration. If fastfile is not used, IOS still remains the default platform.

I tested the PR with these files where several possible scenarios are used.

Testing Steps

Steps are described in the root of the folder whose link is provided above.

milhauscz and others added 27 commits July 24, 2019 09:31
Frameit refactoring and decoupling from the deliver module
New Device class which contains all necessary device information: id, device name, priority, resolutions, screen pixel density, default color and platform
Added "use_platform" parameter which can be specified in Framefile.json, fastfile or passed via CLI parameters; default is iOS for backward compatibility
Added new "android" and "ios" commands for frameit
Updated frames_generator to include Android devices and exclude some deprecated devices
Updated frames_generator's image magick commands to remove shadows and correctly process Android devices (specifically Samsung Galaxy S9 whose screen contains shadows representing round edges)
Updated frameit docs with Android support
Created unit tests for the new Device class
Fixed force_device_type option which didn't work before, also added support to Framefile.json
Added iPhone 8 support
Added new option :use_legacy_iphone7 to prefer iPhone 7 over iPhone 8
Code style fixes according to rubocop
Searching algorithm compares filename with the device's name without "Apple" for backward compatibility
Co-Authored-By: Jan Piotrowski <piotrowski+github@gmail.com>
…wered perceived complexity of config_parser.validate_key)
# Conflicts:
#	frameit/lib/frameit/screenshot.rb
…Facebook devices

Updated device_spec to count with newer iPhone 11 Pro
Added new devices: Google Pixel 4, Google Pixel 4 XL, Samsung Galaxy Note 10, Samsung Galaxy Note 10+, Samsung Galaxy S10, Samsung Galaxy S10+, Xiaomi Mi Mix Alpha, iPhone 11, iPhone 11 Pro, iPhone 11 Pro Max, iPad 10.2, iPad Air 2019, iPad Mini 2019
Fixed name for iPad Pro 12.9 3rd gen.
Fixed device name sanitization in offsets.rb
Created legacy options for iPhone XR, XS and XS Max
…m since it already supports Android; replaced by team_name
…m since it already supports Android; replaced by team_name
… for detecting device frames from fastfile's general defined platform unless configured otherwise.
@joshdholtz
Copy link
Member

@googlebot I consent.

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.

Thanks for adding this! I added one commit to add that current platform logic to a helper but besides that this was 💯 Really appreciate the contribution!

@joshdholtz joshdholtz merged commit 9075249 into fastlane:master May 12, 2020
Copy link

@fastlane-bot fastlane-bot left a comment

Choose a reason for hiding this comment

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

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

@fastlane fastlane locked and limited conversation to collaborators Jul 11, 2020
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

5 participants