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

Create Rake File Tasks #14

Closed
wants to merge 2 commits into from
Closed

Conversation

mkchoi212
Copy link
Contributor

fastlane folder has been created with a simple lane that runs tests for MobileVLCKit; which is empty as of right now :p

@carolanitz
Copy link
Member

I think for MobileVLCKit we don't need to use fastlane and we can work directly with xcodebuild in the circleci yaml

@mkchoi212 mkchoi212 changed the title Setup fastlane Create Rake File Tasks May 23, 2018
@mkchoi212
Copy link
Contributor Author

So instead of using fastlane, I've decided to use rake in order to organize all the required commands to build/test VLCKit; I was looking around the web and got inspired by the folks at PSPDFKit.

These tasks will later be called from CircleCI during the building/testing stages.

@mkchoi212 mkchoi212 force-pushed the fastlane-setup branch 4 times, most recently from d3b8ecf to 7dce59e Compare May 24, 2018 19:30
Rakefile Outdated
SCHEME_IOS = "MobileVLCKit"
PROJECT_IOS = "MobileVLCKit.xcodeproj"

VLC_FLAGS_SIM = "-da x86_64"
Copy link
Member

Choose a reason for hiding this comment

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

If you only use SDK_SIM_DEST, SDK_SIM, VLC_FLAGS_SIM in one place there is no need to define them here.

Copy link
Member

Choose a reason for hiding this comment

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

Do we maybe want to add -v option as well for more info?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@carolanitz I took them out as vars because they are going to be used often later on. i.e. when task for code coverage is added on

Rakefile Outdated
SDK_SIM = "iphonesimulator11.3"
SDK_SIM_DEST = "'platform=iOS Simulator,name=iPhone 7,OS=11.3'"

SCHEME_IOS = "MobileVLCKit"
Copy link
Member

Choose a reason for hiding this comment

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

right now the testing and build scheme are called the same which is very confusing. Can we rename it to also include the name test?

Rakefile Outdated
required_dirs = ["./libvlc/vlc/install-iPhone", "./libvlc/vlc/install-iPhoneSimulator", "./libvlc/vlc/build-iPhoneSimulator"]

if File.exist?(plugin_file) and dirs_exist?(required_dirs)
puts "Found pre-existing build directory. Skipping build"
Copy link
Member

Choose a reason for hiding this comment

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

I actually think you don't need to check for this since the compileAndBuild will be already fast if those directories exist

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmmm on my machine, it takes about 15 minutes to do a build even when those directories exist 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also I'm not 100% how it works yet but I think every time a CI build is triggered, it clones a fresh version of the repo, therefore removing the previous build directories

Copy link
Member

Choose a reason for hiding this comment

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

Yes, that's actually true you would need to build a cache yourself. We would need to make sure that we don't use the cache when we add for example a new patch or change something in the compileAndBuildVLCKit.sh

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If compileAndBuildVLCKit.sh changes, CircleCI will do a rebuild. It's done in .circle.ci/config.yml with this

- save_cache:
    key: libvlc-{{ checksum "compileAndBuildVLCKit.sh" }}

I'm not sure about adding new patches though cause I haven't really figured out how patches work yet :p

Rakefile Outdated
puts "Building VLCKit (iOS Simulator)"

plugin_file = "Resources/MobileVLCKit/vlc-plugins-iPhone.h"
required_dirs = ["./libvlc/vlc/install-iPhone", "./libvlc/vlc/install-iPhoneSimulator", "./libvlc/vlc/build-iPhoneSimulator"]
Copy link
Member

Choose a reason for hiding this comment

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

do you really need iPhone when you just build for the Simulator?

@mkchoi212 mkchoi212 force-pushed the fastlane-setup branch 8 times, most recently from 5c84e8d to 04d6d05 Compare May 25, 2018 19:30

def dirs_exist?(directories)
directories.each do |dir|
return false unless Dir.exist?(dir)
Copy link
Member

Choose a reason for hiding this comment

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

can't you just do this?

return Dir.exist?(dir)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you talking about replacing the for-loop with Dir.exist?(dir_1) && Dir.exist?(dir_2)?

Copy link
Member

Choose a reason for hiding this comment

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

no just replace the return false unless Dir.exist?(dir) with return Dir.exist?(dir) but I actually don't know enough about ruby. So I'm genuinely asking because I don't know

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you do that, the for-loop would return after only evaluating the first element in the directories array.

return false unless Dir.exist?(dir), returns a false if and only if Dir.exist?(dir) evaluates to false.

If the loop doesn't return, the function implicitly returns a true

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I originally had something like

def dirs_exist?(directories)
  directories.each { |dir| 
    if Dir.exist?(x) == false { 
       return false
    }
  }
  return true 

but rubocop kept yelling at me until I got to what I have right now :D

@mkchoi212 mkchoi212 closed this May 28, 2018
@mkchoi212 mkchoi212 reopened this May 28, 2018
@mkchoi212
Copy link
Contributor Author

I just saw that 23515c7 hasn't been merged yet.

@Mikanbu
Copy link
Member

Mikanbu commented May 28, 2018

Seems that it has with cb34d56

@Mikanbu Mikanbu closed this May 28, 2018
@mkchoi212 mkchoi212 deleted the fastlane-setup branch July 25, 2018 01:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants