-
Notifications
You must be signed in to change notification settings - Fork 200
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
feat: add support for Java templates #129
Conversation
Would love to see this get merged |
@friederbluemle Great work on this one, much appreciated 🙏 !
I'm fine with having this one here. As a next improvement step, I'd think about having module type selection divided into categories (select either pure js or native, if native then either functional, view or both etc.), so that we're not lost in there. But that's another topic. WDYT @satya164 ? |
Yes, great point! I also had some ideas regarding this while working on the changes here. Like you said, it's another topic, and I wanted to keep the PR focused. Happy to discuss, help and contribute more in the future! 👍 |
@satya164 - If you have a few minutes can you please check this PR? Thanks! 🙏 |
This PR really helped me out. Thanks! |
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.
Currently using this in prod
9eb2445
to
e4ad75b
Compare
I just rebased this, which turned out to be a bit tricky and quite a bit of work after all the structural changes that got merged (especially dc56fc9). I would very much appreciate it if this could get merged/reviewed soon (before more changes are pushed to the main branch). 🙏 |
Hey, sorry for delay, I have been a bit busy with other stuff and also the pending monorepo reorg so haven't been able to check it. I'll try to check it soon. |
How do we use this before being merged and officially released? |
Pull the git repo, compile it, and run the same CLI commands you would as if you ran it from npm |
e4ad75b
to
e03f339
Compare
You should now have the executable script available in Run it directly from there to create a new
|
I see you removed/renamed you branch java-template, should we use the 'main' branch for this until it is released? |
@true-hamid - The PR has already been merged. So until a new release has been published, you would need to use the default branch ( Note that because a squash commit was used to merge the PR, the individual commits are no longer distinguishable in this repo. If you want to take a look at these, you can use the |
Summary
This PR adds support for Java templates (both native module and native view types).
With this, Bob now supports all four possible combinations:
Commit Details
The PR head branch contains 3 commits with incremental changes (one way dependency), best reviewed in succession.
Move Kotlin templates to separate directory
As the subject line states, this merely renames the existing Kotlin templates directory to
kotlin-library
, similar to the existingobjc-library
/swift-library
categories. No functional changes, but prepares for the next commit with independent precursor work insrc/create.ts
.Add Java templates
All new templates in
templates/java-library
andtemplates/java-view-library
. Additional changes insrc/create.ts
to add the selection to thebob create
command.Combine native and native-view into native-common
This final commit again does not add any new functionality, but removes a large number of duplications by combining
native-library
andnative-view-library
intonative-common
(both on the Android and iOS side). I can move this one to a separate PR if preferred.Test plan
Use
bob create <name>
and select one of the new templates, verify that everything works.Closes #82
Once approved, please use a normal GitHub merge (i.e. NO rebase/squash merge) to integrate the commit(s) from the PR head branch. The changes are broken up into meaningful, atomic commits, and my branch should already be up-to-date with the latest base branch. If it isn't, or if you want me to change anything, please let me know, and I will update the branch as soon as possible. Thank you!