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

neil new: Add support for deps-based templates #51

Merged
merged 25 commits into from
Jul 31, 2022

Conversation

rads
Copy link
Collaborator

@rads rads commented Jul 22, 2022

Usage

Let's say you want to set up a kit-clj project on a new macOS machine. Here's what that looks like without neil:

brew install clojure
clojure -Ttools install com.github.seancorfield/clj-new '{:git/tag "v1.2.381"}' :as new
clojure -Tnew create :template io.github.kit-clj :name yourname/app

And then with neil:

brew install clojure babashka/brew/neil
neil new io.github.kit-clj/kit yourname/app

Testing

Note: To make the above command work temporarily, you need to add the --git/url and --git/sha options to use my fork of kit-clj/kit:

./neil new io.github.kit-clj/kit yourname/app \
  --git/url https://github.com/rads/kit \
  --git/sha cb2236503c5fd5202df28bca1551bd2128a71e23

Once Kit gets support for deps-new (see kit-clj/kit#52), the --git/url and --git/sha options can be removed.

Notes

If you run into an error like this while testing, this is due to hitting the GitHub rate limit:

Error building classpath. Library io.github.rads/kit has coord with missing sha

TODO

  • Add tests
    • Unit test for neil new scratch ...
    • End-to-end test for neil new io.github.rads/neil-new-test-template ...
      • Change io.github.rads/neil-new-test-template to something else?
    • Unit tests for extended options
  • Update docs

@rads rads changed the title WIP - Add support for deps-based templates WIP - neil new: Add support for deps-based templates Jul 22, 2022
src/babashka/neil.clj Outdated Show resolved Hide resolved
; template deps overrides
:local/root
:git/url
:git/sha
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

From the deps-new docs:

:local/root and git-based coordinates are supported, but Maven/Clojars coordinates are not.

tests.clj Outdated Show resolved Hide resolved
@rads
Copy link
Collaborator Author

rads commented Jul 24, 2022

Made some progress on porting Kit's existing clj-new template to deps-new: kit-clj/kit#52 (comment)

neil Outdated
:dry-run

; template deps overrides
:local/root
Copy link
Contributor

Choose a reason for hiding this comment

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

It's not necessary to explicitly list all the options. Babashka CLI automatically parses all command line flags. The list your using here is for converting options in order to options, using :cmds-opts.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is just a note.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Makes sense. I updated the :cmds-opts to be [:template :name :target-dir]. These are the ones that made most sense to me as positional arguments.

For the special named arguments which are not directly supported by deps-new (:git/url, :git/sha, :git/tag, :local/root), we'll probably want to add those to the neil help docs in addition to the existing link to the deps-new override options.

Copy link
Contributor

Choose a reason for hiding this comment

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

On line 16 (on main) we already have a bunch of options. I'd prefer if we could align the names of options, even if they are used in different contexts. Since we already have :sha , maybe we should re-use that one? Or should we deprecate it and rename it to :git/sha (with :sha being an :alias for :git/sha)?

Copy link
Collaborator Author

@rads rads Jul 28, 2022

Choose a reason for hiding this comment

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

@borkdude: Here's the solution I came up with:

  • neil new will always accept namespaced keys like :git/sha
  • neil new also supports the existing :sha and :latest-sha options
    • :sha is simply renamed to :git/sha before parsing
    • :latest-sha provides an alternative to fetching the latest :git/tag (which is the default behavior when neither :git/tag, :git/sha, nor :latest-sha is provided)
  • Any options for neil new that are not already in babashka.neil/spec should be documented specifically for neil new. If we find some neil new options to be useful elsewhere, we can promote them to babashka.neil/spec as needed.

What do you think about this?

neil Outdated Show resolved Hide resolved
@rads
Copy link
Collaborator Author

rads commented Jul 24, 2022

I updated the logic in template-libs so we can fail fast when detecting invalid combinations of deps options, such as :local/root and :git/url at the same time. I split the logic into separate cases since it was getting tricky to implement everything in one place.

@rads
Copy link
Collaborator Author

rads commented Jul 28, 2022

Dev note: Not sure if there's a better way to do this, but here's what I'm currently using to auto-build the neil file during development (using entr):

find . -iname '*.clj' -o -iname 'prelude' | entr -s 'bb gen-script && date'

@rads rads changed the title WIP - neil new: Add support for deps-based templates neil new: Add support for deps-based templates Jul 28, 2022
@rads
Copy link
Collaborator Author

rads commented Jul 28, 2022

@borkdude: Finished up with everything I originally planned to do so far. This is officially ready for review. 🙂

@rads rads marked this pull request as ready for review July 28, 2022 03:37
@rads
Copy link
Collaborator Author

rads commented Jul 29, 2022

I realized I hadn't tested this with clj -M:neil yet and I found some bugs with that. I just pushed up a commit to make the new command work in both bb and clj environments.

One caveat for clj -M:neil usage is that babashka.deps/add-deps is not supported, so custom templates need to be specified explicitly in a deps.edn file or -Sdeps. Seems like a reasonable tradeoff to me since that's how deps-new already works. I think most people are going to want to use the bb environment anyways when they don't want to specify deps.edn:

clojure -M:neil new io.github.rads/neil-new-test-template foo   5.62s user 0.19s system 294% cpu 1.973 total
neil new io.github.rads/neil-new-test-template foo    0.10s user 0.07s system 30% cpu 0.523 total

src/babashka/neil.clj Outdated Show resolved Hide resolved
rads added 3 commits July 29, 2022 16:58
Classpath property must always be set manually when using babashka with
deps-new
"Returns parsed deps-new template deps opts from raw CLI opts."
[cli-opts]
(-> cli-opts
(set/rename-keys {:sha :git/sha})
Copy link
Contributor

Choose a reason for hiding this comment

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

This we can do via the :alias feature of babashka.cli I think?

@borkdude borkdude merged commit 805f578 into babashka:main Jul 31, 2022
@rads rads deleted the new-command2 branch July 31, 2022 12:17
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

2 participants