Add Makefile #49

duartepinto · 2021-03-25T11:48:46Z

Added a Makefile with the purpose of aggregating commonly used commands. Also added a prerequisites script and make command, to make it easier for someone new to bootstrap a new development system.

I could have added some make commands related to publishing the pod/or documentation, but decided not to, as that is still a manual process and those commands would end up not being really useful on their own.

Also removed the pod spec lint from being executed in the tests. This command does not test the current commit, but rather the external repo and associated tag.

environment

According to https://guides.cocoapods.org/making/using-pod-lib-create.html#deploying-your-library `pod spec lint` checks the external repo and associated tag, not the current state of the current commit, so it does not make sense to run this (unless we are going to push a new version)

This is in order to not force an Xcode version to run the code.

thyandrecardoso · 2021-03-26T11:36:52Z

.scripts/prerequisites.sh

+
+if ! which xcodebuild >/dev/null; then
+ echo "xcodebuild does not seem to be installed. You can install it by installing Xcode from the App Store."
+ exit 1


It would be more useful if we could exit only when we've run all the tests. We can print all the messages we want, and only exit in the end.

Done in a698e94

thyandrecardoso · 2021-03-26T11:37:23Z

Makefile

@@ -0,0 +1,44 @@
+XCARGS := -workspace VelocidiSDK.xcworkspace \


Please align these properly.

Done in 6aaf770

Makefile

thyandrecardoso · 2021-03-26T11:51:34Z

Makefile

+all: clean build
+
+clean:
+ set -o pipefail && xcodebuild $(XCARGS) -scheme VelocidiSDK clean | xcpretty


Shouldn't this clean the examples too?

Done in 3d01567

Makefile

thyandrecardoso · 2021-03-26T12:01:56Z

Makefile

@@ -0,0 +1,44 @@
+XCARGS := -workspace VelocidiSDK.xcworkspace \


Please add this file to the xcode group, so that it is visible inside the project.

Done in 323259f

thyandrecardoso · 2021-03-26T12:19:33Z

Makefile

+
+oclint:
+ set -o pipefail && \
+ xcodebuild -scheme VelocidiSDK -sdk iphoneos -workspace VelocidiSDK.xcworkspace clean && \


Can't we save a line here and do xcodebuild -scheme VelocidiSDK -sdk iphoneos -workspace VelocidiSDK.xcworkspace COMPILER_INDEX_STORE_ENABLE=NO clean build instead?

Fixed in 6aaf770

thyandrecardoso · 2021-03-26T12:19:41Z

Makefile

+prerequisites:
+ .scripts/prerequisites.sh
+
+oclint:


I've had some challenges running this.

pyenv: python3: command not found The `python3' command exists in these Python versions: 3.7.9 Note: See 'pyenv help global' for tips on allowing both python2 and python3 to be found. make: *** [oclint] Error 127

dlopen(/usr/local/bin/../lib/oclint/rules/libTooManyMethodsRule.dylib, 1): no suitable image found. Did find: /usr/local/bin/../lib/oclint/rules/libTooManyMethodsRule.dylib: code signature in (/usr/local/bin/../lib/oclint/rules/libTooManyMethodsRule.dylib) not valid for use in process using Library Validation: library load disallowed by system policy oclint: error: cannot open dynamic library: /usr/local/bin/../lib/oclint/rules/libTooManyMethodsRule.dylib make: *** [oclint] Error 1

I don't think I ever had this problem. I tried uninstalling and installing oclint and I could not replicate this issue. I don't use pyenv, so it might be related to that.

Regardless of pyenv, I think python3 is required.

The second problem is not related to python.

thyandrecardoso · 2021-03-26T12:20:00Z

Makefile

+prerequisites:
+ .scripts/prerequisites.sh
+
+oclint:


Shouldn't this oclint run also for the examples?

Added a command for this in 3025b2c

thyandrecardoso · 2021-03-26T12:21:52Z

.scripts/prerequisites.sh

+ exit 1
+fi
+
+if ! which xcpretty >/dev/null; then


These tools are all mostly dead :(
The project seems mostly dead: xcpretty/xcpretty#360

Co-authored-by: André Cardoso <[email protected]>

codecov · 2021-03-26T12:53:36Z

Codecov Report

Merging #49 (173f363) into master (601faf8) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master      #49   +/-   ##
=======================================
  Coverage   92.71%   92.71%           
=======================================
  Files          54       54           
  Lines         728      728           
=======================================
  Hits          675      675           
  Misses         53       53

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 601faf8...173f363. Read the comment docs.

Move clean instruction.

thyandrecardoso · 2021-03-30T13:00:14Z

Makefile

+ -destination "platform=iOS Simulator,OS=$(TEST_SDK),name=$(TEST_DEVICE)" \
+ GCC_INSTRUMENT_PROGRAM_FLOW_ARCS=YES GCC_GENERATE_TEST_COVERAGE_FILES=YES CODE_SIGN_IDENTITY="" CODE_SIGNING_REQUIRED=NO
+
+all: clean build


Nitpick: I think that it is most common to have the default target be only the build phase. This allows one to take advantage of an incremental compilation while repeatedly calling make.

clean is normally called separately.

Yes, that makes more sense! Fixed in 173f363

thyandrecardoso · 2021-03-30T13:44:42Z

Makefile

+build:
+ set -o pipefail && xcodebuild $(XCARGS) -scheme VelocidiSDK build | xcpretty
+
+test:


Shouldn't the test target depend on the build target?

Yes. Fixed in 173f363

thyandrecardoso · 2021-03-30T13:46:22Z

Makefile

+ pod install --project-directory=Examples/
+
+build-objc-example:
+ set -o pipefail && xcodebuild $(XCARGS) -scheme ObjcExample clean build | xcpretty


Shouldn't these examples depend on the install-examples target?

Fixed in 173f363

…'build'. 'build-...-examples' targets depend on 'install-examples'.

thyandrecardoso

Looks fine.

In the future, I think we should try to actually force the use of oclint which is, right now, an optional step of the build. Unlike swiftlint that runs as part of the compilation step, we don't force oclint to run anywhere. It would be nice if we could make a better use of it even automate its recommendations.

We also have some other steps that might be integrated with the Makefile, such as the documentation build and publishing steps.

duartepinto added 7 commits March 24, 2021 17:18

Add Makefile

4e68b4b

Add prerequisites script to help with bootstrapping a new development

9c7169d

environment

Build examples with make

137a26f

Github actions flow uses make when build examples

2d6dc7e

Add echo at end of prerequisites script

0f6ee94

Use default 'iphonesimulator' sdk

ebfe3c9

This is in order to not force an Xcode version to run the code.

duartepinto requested a review from thyandrecardoso March 25, 2021 11:48

duartepinto self-assigned this Mar 25, 2021

thyandrecardoso requested changes Mar 26, 2021

View reviewed changes

duartepinto and others added 3 commits March 26, 2021 12:50

Update Makefile

afe8a01

Co-authored-by: André Cardoso <[email protected]>

Update Makefile

7d7cac1

Co-authored-by: André Cardoso <[email protected]>

Update Makefile

eb0cfff

Co-authored-by: André Cardoso <[email protected]>

duartepinto added 8 commits March 26, 2021 13:04

Make prerequisites fail only after validating all

a698e94

Add files to xcode group

323259f

Align code.

6aaf770

Move clean instruction.

'clean' make command cleans all schemes

3d01567

Oclint examples

3025b2c

install-examples first builds VelocidiSDK

7da4ecc

Remove set pipefail in clean command

6bfeb63

Use makefile with env vars for CI tests

9160da5

duartepinto requested a review from thyandrecardoso March 29, 2021 14:27

thyandrecardoso requested changes Mar 30, 2021

View reviewed changes

'build' is default target for make command. 'test' target depends on …

173f363

…'build'. 'build-...-examples' targets depend on 'install-examples'.

duartepinto requested a review from thyandrecardoso March 30, 2021 14:01

thyandrecardoso approved these changes Mar 31, 2021

View reviewed changes

thyandrecardoso merged commit f4b1698 into master Mar 31, 2021

thyandrecardoso deleted the makefile branch March 31, 2021 08:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Makefile #49

Add Makefile #49

duartepinto commented Mar 25, 2021

thyandrecardoso Mar 26, 2021

duartepinto Mar 26, 2021

thyandrecardoso Mar 26, 2021

duartepinto Mar 26, 2021

thyandrecardoso Mar 26, 2021

duartepinto Mar 26, 2021

thyandrecardoso Mar 26, 2021

duartepinto Mar 26, 2021

thyandrecardoso Mar 26, 2021

duartepinto Mar 26, 2021

thyandrecardoso Mar 26, 2021

duartepinto Mar 26, 2021

thyandrecardoso Mar 26, 2021

thyandrecardoso Mar 26, 2021

duartepinto Mar 29, 2021

thyandrecardoso Mar 26, 2021

codecov bot commented Mar 26, 2021 •

edited

thyandrecardoso Mar 30, 2021

duartepinto Mar 30, 2021

thyandrecardoso Mar 30, 2021

duartepinto Mar 30, 2021

thyandrecardoso Mar 30, 2021

duartepinto Mar 30, 2021

thyandrecardoso left a comment •

edited

		@@ -0,0 +1,44 @@
		XCARGS := -workspace VelocidiSDK.xcworkspace \

Add Makefile #49

Add Makefile #49

Conversation

duartepinto commented Mar 25, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Mar 26, 2021 • edited

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thyandrecardoso left a comment • edited

Choose a reason for hiding this comment

codecov bot commented Mar 26, 2021 •

edited

thyandrecardoso left a comment •

edited