-
Notifications
You must be signed in to change notification settings - Fork 93
Make 'system-map' work with k/v pairs or a map #77
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
base: master
Are you sure you want to change the base?
Make 'system-map' work with k/v pairs or a map #77
Conversation
This requires bumping min Clojure version to 1.11
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.
Pull Request Overview
This PR modifies the system-map function to accept either keyword arguments or a map as input, and updates the Clojure dependency version from 1.7.0 to 1.11.0.
- Changed the
system-mapfunction signature from accepting variadic key-value pairs to using destructured keyword arguments - Updated Clojure dependency from version 1.7.0 to 1.11.0
- Added a test case for passing a map directly to
system-map
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/com/stuartsierra/component.cljc | Modified system-map function to accept keyword arguments or a map, updated docstring, removed validation logic |
| test/com/stuartsierra/component_test.clj | Added test case for new map-based system-map API |
| project.clj | Updated Clojure dependency from 1.7.0 to 1.11.0 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Sorry for the AI noise - looks like something was enabled on my end. |
|
TBH, I think something more useful would be a new function that took var args and broken them into triples: name, component, dependencies. |
Can you expand a bit? Like a combination of system-map and system-using? |
|
Yes, something like |
|
Interesting - I had a similar need of declaratively creating systems, but I went a bit further. I created a library which allows for pure-data definition of component systems: https://github.com/lukaszkorecki/oreo?tab=readme-ov-file#annotated-example it doesn't look that different but not sure if it provides what you're after. |
|
Interesting. I mess around with SystemMaps often enough, but usually I'm working with things that are already SystemMaps. If I had a map of components, but not a SystemMap, I would probably do something like The original |
For as long as I've been working with Component (over a decade now?) I don't think I ever used |
This small change makes
system-mapwork with both key/value pairs and passing the system as a map. A lot of the framework-ish code that I wrote over the years tends to compose systems out of maps, so I never found the use forsystem-mapas is, and instead always usedmap->SystemMap. With this change we can do both and keep the public facing API somewhat cleaner.Only downside is that this makes Component require Clojure 1.11 and up as that's when support for kw-args as map was added.