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 Zattoo Fastlane #38

Merged
merged 9 commits into from Dec 4, 2017

Conversation

Projects
None yet
7 participants
@dcordero
Contributor

dcordero commented Nov 16, 2016

Having these examples available here was something very useful for us to configure our lanes.

That is why we would like to also share our configuration for the next guys coming here to check other's solutions.

dcordero added some commits Nov 16, 2016

#### Setup ####
desc "Fetches development certificates and provisioning profiles"

This comment has been minimized.

@branidiaz

branidiaz Nov 25, 2016

Niceeee 😎

@dcordero dcordero referenced this pull request Dec 14, 2016

Closed

Inclusion of tvOS as supported platform #7491

2 of 2 tasks complete
end
private_lane :setup_preview_display_name do
update_info_plist(plist_path: "Zattoo/Resources/Info.plist", display_name: Zattoo::DisplayName::Preview)

This comment has been minimized.

@pvinis

pvinis Jan 12, 2017

where is this Zattoo::DisplayName::Preview defined? i want to use this too rather than specifying the string everywhere..

This comment has been minimized.

@dcordero

dcordero Jan 12, 2017

Contributor

@pvinis those values are defined in a separate ruby file, imported at top of the Fastfile with:

require './helpers/zattoo.rb'

This ruby file has a format similar to the one below:

class MyCompany

   MyVariable = "Hey dude"

   class MySecondVariable
      Production = "Value For Production"
      QA = "Value for QA"
   end

end

So you could use MyCompany::MyVariable or MyCompany::MySecondVariable::Production in your Fastfile, avoiding the use of literal strings everywhere.

This comment has been minimized.

@janpio

janpio Nov 30, 2017

Contributor

Hey @dcordero could you actually submit examples for these files so the setup is complete?

This comment has been minimized.

@dcordero

dcordero Dec 1, 2017

Contributor

Good point @janpio I have added it with dummy values. It can be definitely useful for somebody else trying to build something similar.

desc "Install Xcode plugins"
lane :install_xcode_plugins do
install_xcode_plugin(github: 'https://github.com/neonichu/FixCode')

This comment has been minimized.

@janpio

janpio Nov 30, 2017

Contributor

This is probably not part of the setup any more as this doesn't work any more, right?

This comment has been minimized.

@KrauseFx

KrauseFx Nov 30, 2017

Member

Sad world we live in :(

This comment has been minimized.

@dcordero

dcordero Dec 1, 2017

Contributor

Indeed, removed !!! 🗑

Cheer up @KrauseFx at least it is Friday !!!

bd6

dcordero added some commits Dec 1, 2017

@googlebot googlebot added the cla: yes label Dec 1, 2017

@janpio

janpio approved these changes Dec 1, 2017

Looking good!

@milch

Thanks for your example ❤️ I especially liked the idea of your zattoo.rb file, don't think I've seen that anywhere else before

If you could just change the match lanes (optionally the testing ones) I'd be in favour of merging 👍

match(type: 'appstore', readonly: true, team_id: Zattoo::TeamId, app_identifier: Zattoo::AppIdentifier::Production)
match(type: 'appstore', readonly: true, team_id: Zattoo::TeamId, app_identifier: Zattoo::TopShelfAppIdentifier::Preview)
match(type: 'appstore', readonly: true, team_id: Zattoo::TeamId, app_identifier: Zattoo::TopShelfAppIdentifier::AppleStore)
match(type: 'appstore', readonly: true, team_id: Zattoo::TeamId, app_identifier: Zattoo::TopShelfAppIdentifier::Production)

This comment has been minimized.

@milch

milch Dec 2, 2017

Member

Might I suggest for this and the previous lane that you change the code to something like this to avoid repetition?

lane :fetch_distribution_certificates do
  [Zattoo::AppIdentifier::Preview, Zattoo::AppIdentifier::AppleStore, Zattoo::AppIdentifier::Production, Zattoo::TopShelfAppIdentifier::Preview, Zattoo::TopShelfAppIdentifier::AppleStore, Zattoo::TopShelfAppIdentifier::Production].each do |app_id|
    match(type: 'appstore', readonly: true, team_id: Zattoo::TeamId, app_identifier: app_id)
  end
end

Alternatively, in your zattoo.rb helper you could define two arrays, e.g. Zattoo::AppIdentifier::AllDevelopment and Zattoo::AppIdentifier::AllDistribution and use those in the lanes:

lane :fetch_dev_certificates do
  match(type: 'development', readonly: true, team_id: Zattoo::TeamId, app_identifier: Zattoo::AppIdentifier::AllDevelopment)
end

This comment has been minimized.

@janpio

janpio Dec 2, 2017

Contributor

I actually like how the repetition makes it very explicit what happens. I would not do a loop here.

This comment has been minimized.

@milch

milch Dec 2, 2017

Member

I thought about that, but all the match parameters are the same except for the app identifier. Imo it doesn't really add any readability to have the explicit calls to match and I don't see any other advantages.

The disadvantages I can see are that it's easy to forget a single app identifier (say, they add a watch extension) in that long list, and when a parameter gets added/changed it's easy to forget one in the middle of the 7 calls.

Even if the company behind this has procedures in place to make sure these things never happen, since this is an example that should show other people getting started "how it's done", I would not generally recommend this style.

I also just remembered that the loop isn't even necessary since app_identifier actually supports arrays as parameters 😄 I'll update the previous comment

This comment has been minimized.

@janpio

janpio Dec 2, 2017

Contributor

I personally would still leave it the way it is.

No matter if it is a loop (which many people working with Fastfile probably don't know as they have no idea of ruby) or "only" an array, both is not as explicit as seeing 4 lines of the same call, just with changing app identifiers.

This comment has been minimized.

@dcordero

dcordero Dec 2, 2017

Contributor

I also prefer them explicitly listed, it makes it more obvious, but I do also see the point on avoiding the repetition on the match parameters...

So another option would be to keep the lane explicitly using each app identifier but creating a private lane to avoid the repetition on the parameters when using match:

lane :fetch_dev_certificates do
  run_match(type: 'development', app_identifier: Zattoo:AppIdentifier::Production)
  run_match(type: 'development', app_identifier: Zattoo:TopShelfAppIdentifier::Production)
end

desc "Fetches distribution certificates and provisioning profiles"
lane :fetch_distribution_certificates do
  run_match(type: 'appstore', app_identifier: Zattoo:AppIdentifier::Preview)
  run_match(type: 'appstore', app_identifier: Zattoo:AppIdentifier::AppleStore)
  run_match(type: 'appstore', app_identifier: Zattoo:AppIdentifier::Production)
  run_match(type: 'appstore', app_identifier: Zattoo:TopShelfAppIdentifier::Preview)
  run_match(type: 'appstore', app_identifier: Zattoo:TopShelfAppIdentifier::AppleStore)
  run_match(type: 'appstore', app_identifier: Zattoo:TopShelfAppIdentifier::Production)
end

private_lane :run_match do |options|
    type = options[:type]
    app_identifier = options[:app_identifier]
    match(type: type, readonly: true, team_id: Zattoo::TeamId, app_identifier: app_identifier)
end

What do you think?

This comment has been minimized.

@milch

milch Dec 4, 2017

Member

I also prefer them explicitly listed, it makes it more obvious

Fair enough. I don't think it's necessary in this case since it doesn't really "fix" the repetition. Since you showed how to avoid repetition in this commit 389bb2c maybe just place a comment before the match calls that explains why? Something like

# All app identifiers are passed to separate match calls to make it easier to see what is happening
lane :ui_tests do
cocoapods(repo_update: true)
scan(scheme: "ZattooUITests")
post_to_slack(message: "*UI Tests*: All tests successfully passed", success: true)

This comment has been minimized.

@milch

milch Dec 2, 2017

Member

Same here, I'd rather repetition be avoided:

private_lane :run_tests do |opts|
  cocoapods(repo_update: true)
  scan(scheme: opts[:scheme])
  post_to_slack(message: "*#{opts[:message]} Tests*: All tests successfully passed", success: true)
end

lane :tests do
  run_tests(message: "Unit, UI, and Top Shelf")
end

lane :unit_tests do
  run_tests(message: "Unit", scheme: "ZattooTests")
end

lane :ui_tests do
  run_tests(message: "UI", scheme: "ZattooUITests")
end

Although it's not as big of a deal with these than with the match lanes, so you decide which way fits your workflow better

This comment has been minimized.

@dcordero

dcordero Dec 2, 2017

Contributor

👍 I like this, definitely much better.

Changed

@@ -0,0 +1,34 @@
#!/usr/bin/env sh

This comment has been minimized.

@milch

milch Dec 2, 2017

Member

Have you thought about making this a custom action or plugin? I just checked, there is no webtranslate plugin yet, so other people might be interested in this

@milch

milch approved these changes Dec 4, 2017

@janpio janpio merged commit 072f0f5 into fastlane:master Dec 4, 2017

1 check passed

cla/google All necessary CLAs are signed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment