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

Define Archive Action on Scheme #529

Closed
wants to merge 8 commits into from
Closed

Define Archive Action on Scheme #529

wants to merge 8 commits into from

Conversation

grdsdev
Copy link
Contributor

@grdsdev grdsdev commented Oct 3, 2019

Short description πŸ“

The purpose of this PR is to add support for defining a custom archive action when defining a scheme, so that we're able to provide a configuration when running the archive job.

Solution πŸ“¦

The solution I propose is to follow the same implementation that is already being used on the other action's definitions.

Implementation πŸ‘©β€πŸ’»πŸ‘¨β€πŸ’»

  • Create class ArchiveAction
  • Add archiveAction attribute to Scheme class
  • Implement method for generating a XCScheme.ArchiveAction from an ArchiveAction
  • Implement unit tests
  • Update documentation

@tuistbot
Copy link
Contributor

tuistbot commented Oct 3, 2019

2 Warnings
⚠️ Please include a CHANGELOG entry.
You can find it at CHANGELOG.md.
⚠️ Have you introduced any user-facing changes? If so, please take some time to update the documentation. Keeping the documentation up to date makes it easier for users to learn how to use Tuist.

Generated by 🚫 Danger

@codecov
Copy link

codecov bot commented Oct 3, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@863cb8a). Click here to learn what that means.
The diff coverage is 33.33%.

Impacted file tree graph

@@           Coverage Diff            @@
##             master    #529   +/-   ##
========================================
  Coverage          ?   91.5%           
========================================
  Files             ?     358           
  Lines             ?   19373           
  Branches          ?       0           
========================================
  Hits              ?   17727           
  Misses            ?    1646           
  Partials          ?       0
Impacted Files Coverage Ξ”
...neratorTests/Generator/SchemesGeneratorTests.swift 100% <100%> (ΓΈ)
Sources/TuistGenerator/Models/Scheme.swift 41.42% <12.5%> (ΓΈ)
...es/TuistGenerator/Generator/SchemesGenerator.swift 96.07% <37.5%> (ΓΈ)

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 863cb8a...ffded53. Read the comment docs.

@pepicrft
Copy link
Contributor

pepicrft commented Oct 3, 2019

Thanks for bringing this up @GRSouza. The PR is looking good πŸ‘

@grdsdev grdsdev changed the title [WIP] Define Archive Action on Scheme Define Archive Action on Scheme Oct 4, 2019
@grdsdev
Copy link
Contributor Author

grdsdev commented Oct 4, 2019

Hi @pepibumur I think I'm done with the implementation, anything else I could do?

Copy link
Collaborator

@ollieatkinson ollieatkinson left a comment

Choose a reason for hiding this comment

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

Looks like a welcome change, πŸ‘.

I left a few comments, some are superficial so feel free to adhere to those as you see fit. Once you're happy with any changes or you would like me to review it again just ping back and I'll take another look and we can look at getting this merged.

Sources/TuistGenerator/Models/Scheme.swift Outdated Show resolved Hide resolved
docs/usage/project.swift.mdx Outdated Show resolved Hide resolved
docs/usage/project.swift.mdx Outdated Show resolved Hide resolved
docs/usage/project.swift.mdx Outdated Show resolved Hide resolved
shared: true,
buildAction: BuildAction(targets: ["App"]),
runAction: RunAction(configurationName: "Beta", executable: "App"),
archiveAction: ArchiveAction(configurationName: "Beta"))
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we update a fixture test to ensure the archive scheme can execute successfully?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it needs to be updated, how do I run the fixtures with my version of tuist? Or is another thing that needs to change?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Our fixtures are defined in fixtures/ and are exexuted by our cucumber test's.

To run them locally you would execute features from the Rakefile:

bundle exec rake features

Inside the generate feature you should be able to specify which action you would like Xcode to run as part of your scheme.

An example of this would be you can define a step in the cucumber tests as follows:

Then I should be able to archive for iOS the scheme App

If you are unsure of any of the above or would like me to step in then please ask

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, but I don't know how this actually works. I wrote a new step as you suggested, but I'm getting the error error: use of unresolved identifier 'ArchiveAction' when a run bundle exec rake features, I guess it is because the tests aren't being run with my version of tuist.

docs/usage/project.swift.mdx Outdated Show resolved Hide resolved
Sources/TuistGenerator/Generator/SchemesGenerator.swift Outdated Show resolved Hide resolved
Copy link
Contributor

@pepicrft pepicrft left a comment

Choose a reason for hiding this comment

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

After addressing @ollieatkinson's comment I think we are good to merge.

@pepicrft
Copy link
Contributor

@GRSouza notice that the API that is available from the Project.swift is defined in the ProjectDescription framework. If you look at this file you'll see that we are not exposing any archive action and therefore you can't use it from a Project.swift file. You'll need to define the models there and extend the generation logic to map from ProjectDescription.ArchiveAction to TuistGenerator.ArchiveAction. You can use this as a reference:

Copy link
Contributor

@pepicrft pepicrft left a comment

Choose a reason for hiding this comment

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

Also, don't forget to rebase from master.

@pepicrft
Copy link
Contributor

Is it ok if I finalize this one @GRSouza?

@grdsdev
Copy link
Contributor Author

grdsdev commented Nov 18, 2019

@pepibumur it's okay, you can finalize it

@pepicrft pepicrft mentioned this pull request Nov 19, 2019
5 tasks
@pepicrft
Copy link
Contributor

I'm closing this in favor of #697, otherwise I can't push code to the branch.

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.

None yet

4 participants