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

Updates to Workflow Templates #100

Merged
merged 4 commits into from
Oct 5, 2021

Conversation

monishsyed
Copy link
Contributor

@monishsyed monishsyed commented Sep 6, 2021

  • Added ability to optionally create a worker or avoid the worker noise completely if your workflow isn't doing any sync work.
  • Worker template now supports both RxSwift as well as ReactiveSwift

Screen Shot 2021-09-07 at 1 26 54 AM

* Worker template is now separate then workflow template that gives us the flexibility to use RxSwift or ReactiveSwift
…and with ReactiveSwift Worker. Also creating symlink for workflow template to avoid duplication
@@ -0,0 +1 @@
/Users/monishsyed/Development/Personal/workflow-swift/Tooling/Templates/Workflow (Verbose).xctemplate/Default/___FILEBASENAME___Workflow.swift
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this file? The pathname should be relative.

Copy link
Contributor Author

@monishsyed monishsyed Sep 8, 2021

Choose a reason for hiding this comment

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

This is the symlink for the original file that resides under Tooling/Templates/Workflow (Verbose).xctemplate/Default/. Even though I had created the symlink with the relative path, seems like symlinks cannot resolve relative paths and so symlinks have to have absolute path ALWAYS.

Why was symlink created in the first place?

It was created to overcome a limitation that we have with xcode files templates i.e files templates cannot be inherited from other templates (project templates can have ancestors though but not file templates) and therefore when creating a template that has RxSwift/ReactiveSwift worker enabled, both ___FILEBASENAME___Workflow.swift and __FILEBASENAME___Worker.swift has to be provided inside generateWorkerRxSwift and generateWorkerReactiveSwift. This leaves us in a situation where ___FILEBASENAME___Workflow.swift will have 3 identical copies and any changes to ___FILEBASENAME___Workflow.swift would require updates to all 3. Symlinks was one solution since it allows you to create aliases for these files. But as you can see, symlinks don't work with relative paths unfortunately :(

What are other options to avoid duplication of ___FILEBASENAME___Workflow.swift?

  1. Maybe just have one copy ___FILEBASENAME___Workflow.swift in Default and run copy commands as part of install-xcode-templates.sh script that copies it in generateWorkerRxSwift and generateWorkerReactiveSwift. Downside is without running the script, the template is pretty much broken. Ideally script should only be there to help you install templates.
  2. Generate the full templates through scripts. May need a bit of work there.
  3. Keep 3 copies of ___FILEBASENAME___Workflow.swift if we don’t anticipate changes to the templates often. I think it should ok for 1.0.0.

What do you all think?

P.S: For workflow template to work correctly, the file/directory structure should look like this

Screen Shot 2021-09-08 at 6 18 04 PM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Went ahead and added back copies of ___FILEBASENAME___Workflow.swift for now

Copy link
Contributor

@dhavalshreyas dhavalshreyas left a comment

Choose a reason for hiding this comment

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

This looks great, thanks for doing this @monishsyed!

At some point, we should come back and add tests here to make sure we're not breaking something as we iterate.

@dhavalshreyas dhavalshreyas merged commit 154e4cf into square:main Oct 5, 2021
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