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

Alfmob 33 implemented ci cd #7

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

naveenmindera
Copy link

No description provided.

@naveenmindera naveenmindera marked this pull request as ready for review November 22, 2024 09:52
Copy link
Member

@p4checo p4checo left a comment

Choose a reason for hiding this comment

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

Nice one! Still some pending questions though 🤓

Alfie/.github/workflows/alfie.yml Outdated Show resolved Hide resolved
Alfie/.github/workflows/alfie.yml Outdated Show resolved Hide resolved
Alfie/.github/workflows/alfie.yml Outdated Show resolved Hide resolved
Alfie/.github/workflows/alfie.yml Outdated Show resolved Hide resolved
Alfie/.github/workflows/alfie.yml Show resolved Hide resolved
Alfie/.github/workflows/alfie.yml Outdated Show resolved Hide resolved
Alfie/.github/workflows/alfie.yml Outdated Show resolved Hide resolved
Alfie/.github/workflows/alfie.yml Outdated Show resolved Hide resolved
Comment on lines +1020 to +1035
FCA0B27B2CEF3EE7000339F6 /* .github */ = {
isa = PBXGroup;
children = (
FCA0B27D2CEF3EFB000339F6 /* workflows */,
);
path = .github;
sourceTree = "<group>";
};
FCA0B27D2CEF3EFB000339F6 /* workflows */ = {
isa = PBXGroup;
children = (
FCA0B27E2CEF3F12000339F6 /* alfie.yml */,
);
path = workflows;
sourceTree = "<group>";
};
Copy link
Member

Choose a reason for hiding this comment

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

There's no need to add the .github folder to the .xcodeproj, neither the alfie.yml file

Copy link
Member

Choose a reason for hiding this comment

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

I think the idea is that we can edit these files while working on the project in Xcode, instead of having to open another editor.

To your point, I think we might not need to add the whole folder and just add the relevant files.

IMO we just need to be careful that these files aren't added to the app target, but I don't see any problem in adding them to the project as references. In other projects I've worked on we even create a group without folder, and add file references to it for all these CI/tooling/setup/documentation files

Copy link
Member

@p4checo p4checo left a comment

Choose a reason for hiding this comment

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

We're on the right path, but still some rough edges here and there 😄

Alfie/.github/workflows/alfie.yml Outdated Show resolved Hide resolved
Alfie/.github/workflows/alfie.yml Outdated Show resolved Hide resolved
Alfie/fastlane/Appfile Outdated Show resolved Hide resolved
Alfie/.github/workflows/alfie.yml Outdated Show resolved Hide resolved
Alfie/.github/workflows/alfie.yml Outdated Show resolved Hide resolved
Copy link
Member

@p4checo p4checo left a comment

Choose a reason for hiding this comment

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

nice additions! We're on the right path 😉

lane :validate_branch do
sh("swiftlint") # Runs SwiftLint
sh("xcodebuild analyze -scheme Alfie -destination '#{ENV['IOS_DESTINATION']}'")
sh("xcodebuild test -scheme Alfie -destination '#{ENV['IOS_DESTINATION']}'")
Copy link
Member

Choose a reason for hiding this comment

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

perhaps use run_tests action (aka scan) ?

https://docs.fastlane.tools/getting-started/ios/running-tests/

We can also set SCAN_DESTINATION and any other env var once which then gets passed automatically to each fastlane action. Take a look here for scan's parameters, for instance.

desc "Push a new beta build to TestFlight"
desc "Validate branch with linting, analysis, and tests"
lane :validate_branch do
sh("swiftlint") # Runs SwiftLint
Copy link
Member

Choose a reason for hiding this comment

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

there's a swiftlint fastlane action. I have mixed feelings about bundling swiftlint executable in the project, but I tend to prefer it being povided externally (e.g. via brew, SPM plugin, ...)

increment_build_number(xcodeproj: "Alfie.xcodeproj")
build_app(scheme: "Alfie")
upload_to_testflight
api_key = app_store_connect_api_key(
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need to store the api_key, as this action stores it on the environment for subsequent actions to use (such as upload_to_testflight, match, etc)


match(type: "appstore", readonly: false, api_key: api_key)

increment_build_number
Copy link
Member

Choose a reason for hiding this comment

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

we are incrementing the build number, but how is it updated after each build?

Shouldn't we get the last tag's value, extract the BN, then increment and set it on the Xcode project instead?


increment_build_number

version = get_version_number(xcodeproj: "Alfie.xcodeproj")
Copy link
Member

Choose a reason for hiding this comment

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

we can and probably should store the Xcode project's path on an env var, which avoids duplication and possible errors.

Alfie/.github/workflows/alfie.yml Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants