-
Notifications
You must be signed in to change notification settings - Fork 3
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
Prepare core executions #10
Conversation
ab8e695
to
d0f827e
Compare
adb4ffb
to
eded5ef
Compare
This reverts commit d0f827e.
Tuist is handy, and more fit rusty style.
e5dc5c5
to
66d596e
Compare
Please review, @akesson. Thank you. If you have any suggestions, please point them out, and I will address them. |
d6ebad6
to
3af6ba1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code is great. A really nice addition.
The tuist example readme needs improvements. See comments.
I also see that there's a lot of temporary stuff generated (*.xcworspace, .*xcproject, Derived, target etc). Are there any convenient way to remove all temporary cache stuff? Could you add instructions for it in a readme?
In any case: really great stuff!
Oh, and sorry for not reviewing it before. I have a lot of things on my plate... I'll try to be faster. |
@akesson Thanks for your comments, I've updated them. Please review again. 🙏 |
921ac2d
to
d1e8cda
Compare
d1e8cda
to
e9128b1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's still things in the tuist readme that doesn't work, but that's ok. As you pointed out, this is with focus on the core changes.
Just one code comment I did that I'm not sure if you missed, except for that one it's good to go.
Yes, I've checked the README, and it worked locally on my machine. However, since this is my first time using Tuist, I might have overlooked some requirements. This is a starting point to enhance support for more platforms seamlessly. We will continue to fix and improve these aspects, including tests that are not working on CI yet🤦.
I have responded to that comment. Please review my response to see if I misunderstood anything. If so, I will continue to iterate. |
Sorry for that confusion, regarding the comment without reply, that was in reality already fixed. I was too tired to work yesterday... A big thank you for this contribution and already looking forwards to the next one 😄 |
Fixes #11
General core logic part is initially ready, include
warp library to framework
to support dynamic link for more platforms.cmds
to use core functions.More roadmap: #7