kit-clj / kit-clj.github.io

Kit documentation
https://kit-clj.github.io/
20 stars 27 forks source link

Properly resolve query-fn #53

Closed gerdint closed 8 months ago

gerdint commented 8 months ago

DItto.

I don't like this extra complexity in the beginners tutorial but it is what it is.

yogthos commented 8 months ago

Yeah, it's a just a bit of extra magic. :)

gerdint commented 8 months ago

Would have been nice if Integrant somehow did this automatically but that's the price we pay for the system definition just being a map I guess.

yogthos commented 8 months ago

yup

gerdint commented 8 months ago

@yogthos Actually, in practice the modification times could be stored as a defonced atom the kit.edge.db.sql.conman namespace instead of inside the component instance. This would preclude running several instances of a system including that component in the same REPL session, but is that goal for Kit?

I have actually never seen an example of that in docs or examples I think, but would be nice for stuff like bringing up an additional system with a test profile when running tests from the REPL. Is that suppose to work?

yogthos commented 8 months ago

I'd rather not put hidden state in component libraries. It would provide a bit of convenience, but I don't think it's worth it overall. In general, I'd like to keep Kit as unopinionated as possible, and I can definitely see uses for having multiple systems for things like tests.

gerdint commented 8 months ago

@yogthos I agree, which is obviously why I coded the patch as such.

I do find the testing page a bit light on details. Perhaps a section on such a use case could be described?

yogthos commented 8 months ago

oh yeah, what if we just use metadata instead of sticking it into a map?

yogthos commented 8 months ago

and yeah testing page could definitely be fleshed out more :)

yogthos commented 8 months ago

ok, switched to use meta and seems to work https://github.com/kit-clj/kit/commit/f8a94c19620dee9f0314ed647afb0296a3b65695#diff-51ac4e96c346210de813ab1b0635a210dc4c10a8c0356f79ad3e6d34f9451978

gerdint commented 8 months ago

Very sweet indeed! I am inclinded to ping James Reeves about this trick. I feel it makes resolve-key all but redundant, and should at least be mentioned in the Integant README.

yogthos commented 8 months ago

Yeah it's probably worth mentioning, this is seems like the exact use case metadata is meant for.

gerdint commented 8 months ago

Created issue https://github.com/weavejester/integrant/issues/106

tors 18 jan. 2024 kl. 12:48 skrev Dmitri Sotnikov @.***

:

Yeah it's probably worth mentioning, this is seems like the exact use case metadata is meant for.

— Reply to this email directly, view it on GitHub https://github.com/kit-clj/kit-clj.github.io/pull/53#issuecomment-1898329768, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAADU5Q4MC3CWPYJO7MUISLYPEDZZAVCNFSM6AAAAABB5BDPKOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOJYGMZDSNZWHA . You are receiving this because you authored the thread.Message ID: @.***>