<FromGitter>
<AhsenBaig_gitlab> Updated it to 19. Doing shards update.
<FromGitter>
<AhsenBaig_gitlab> Ok that finally worked thank you. I would suggest that the main pet-tracker app be updated for new users.
<FromGitter>
<drujensen> Will do. We will release soon.
<FromGitter>
<AhsenBaig_gitlab> Awesome! Thank you once again.
<FromGitter>
<elorest> @drujensen 0.30.1 worked a few days ago when I tested it, after the crystal-db, micrate issue. Has something else changed?
<FromGitter>
<elorest> @AhsenBaig_gitlab I'm trying to track down this issue. Nothing in amber 0.30.1 should be requiring `crystal-db` 0.7. I wonder what changed.
<FromGitter>
<drujensen> @elorest ok, I deleted 0.17.3 tagged release and released 0.18.0
<FromGitter>
<drujensen> hopefully that will fix current users
<FromGitter>
<drujensen> we will need to bump the version for amber master to work
<FromGitter>
<Blacksmoke16> @drujensen @elorest @eliasjpr @robacarp opinions on having users define their own initialize methods for Granite modles?
<FromGitter>
<Blacksmoke16> it would ofc be a massive breaking change, but it would allow for things to work in a more crystalesque way.
<FromGitter>
<Blacksmoke16> i.e. we could get rid of `#set_attributes`, compile time errors if you try to do like `Book.new(title: 19)` where `title` is defined as a `String`
<FromGitter>
<Blacksmoke16> main downside would obs be more responsibility on the user, require them to define the initializer for all their models
<FromGitter>
<Blacksmoke16> might look into if there is a better way to automatically define the initializers, to still allow getting rid of `#set_attributes`, but we'll see
<FromGitter>
<drujensen> @Blacksmoke16 I would be ok with creating a different initializer that takes params hash instead of `new`
<FromGitter>
<drujensen> that way in Amber, we can replace `Post.new params` with something like `Post.build params`
<FromGitter>
<drujensen> or in Amber, we handle the conversion of param values to types and remove this completely from Granite
<FromGitter>
<Blacksmoke16> im more in favor of the latter
<FromGitter>
<Blacksmoke16> thats essentially the only custom thing that is in `#set_attributes`
<FromGitter>
<Blacksmoke16> if we are able to ditch that, then it should be easier to define initializers for fields that take advantage of the compile time errors
<FromGitter>
<drujensen> well, we will always have runtime errors since http doesn’t support types. ;-)
<FromGitter>
<drujensen> but if someone was using Granite outside of Amber, I can see the desire to move this out
<FromGitter>
<drujensen> @elorest @eliasjpr any concerns with this change?
<FromGitter>
<Blacksmoke16> however i would think that the `from_params` would live inside of Amber
<FromGitter>
<Blacksmoke16> as an ext to Granite for HTTP stuff
<FromGitter>
<drujensen> why?
<FromGitter>
<drujensen> we added from_json
<FromGitter>
<drujensen> why not `from_params`?
<FromGitter>
<Blacksmoke16> from_json just kinda works with the stdlib, id also be open to removing the serializable stuff by default and if the user wants it they can just include it
<FromGitter>
<drujensen> oh, yes. i’m ok with modularizing it
<FromGitter>
<robacarp> I'm in favor as well
<FromGitter>
<drujensen> not sure it requires a separate shard though
<FromGitter>
<Blacksmoke16> separate shard?
<FromGitter>
<Blacksmoke16> im not too familiar with how it works now but i was just picturing amber reopneing Granite::Base and adding the method
<FromGitter>
<Blacksmoke16> the former would prob be easier to maintain, as it could be tested and stuff
<FromGitter>
<drujensen> well, I don’t know why this wouldn’t be in granite
<FromGitter>
<drujensen> but maybe i’m missing something
<FromGitter>
<Blacksmoke16> thinking about it more i tend to agree
<FromGitter>
<Blacksmoke16> but maybe as an ext, like `require "granite/ext/http"`?
<FromGitter>
<Blacksmoke16> eh idk
<FromGitter>
<drujensen> not following
<FromGitter>
<drujensen> ooohhh
<FromGitter>
<drujensen> no,no
<FromGitter>
<drujensen> so we will only accept a hash in the `from_params`
<FromGitter>
<drujensen> not the Http::Params
<FromGitter>
<drujensen> I see why you are concerned
<FromGitter>
<drujensen> maybe `from_hash` is better
<FromGitter>
<drujensen> Amber can do `params.to_h`
<FromGitter>
<drujensen> does that help?
<FromGitter>
<Blacksmoke16> what is the type of `params` tho?
<FromGitter>
<drujensen> a hash
<FromGitter>
<drujensen> wait
<FromGitter>
<drujensen> sorry, what do you mean?
<FromGitter>
<Blacksmoke16> if its already a hash, do you not have to call `.to_h` on it? or was that just a typo?
<FromGitter>
<drujensen> checking...
<FromGitter>
<drujensen> I think params is an Http::Params object
<FromGitter>
<Blacksmoke16> i think that would be preferable no?
<FromGitter>
<Blacksmoke16> but im not sure what the crystal way of handling stuff like this should be
<FromGitter>
<drujensen> Ok, granite should not be dependent on Http
<FromGitter>
<drujensen> or anything related to web
<FromGitter>
<drujensen> so the contract between Amber and Granite should be passing by a generic data structure
<FromGitter>
<drujensen> like Array or Hash
<FromGitter>
<drujensen> or Json
<FromGitter>
<drujensen> do you agree?
<FromGitter>
<Blacksmoke16> i need to think about how all this would tie together
<FromGitter>
<Blacksmoke16> way im seeing this now is granite should be basically just a fancy class that interacts with a database
<FromGitter>
<Blacksmoke16> extra features, like serialization, should be able to be included, i.e. `Include JSON::Serializable`, or `include MySerializer`
<FromGitter>
<drujensen> yes, single responsibility
<FromGitter>
<drujensen> could be separate shards but yes
<FromGitter>
<drujensen> Granite extensions?
<FromGitter>
<Blacksmoke16> the extension part im not sure how to go about yet
<FromGitter>
<drujensen> probably best in the granite shard
<FromGitter>
<drujensen> ok, forget that for now
<FromGitter>
<Blacksmoke16> to wrap this up id be ok with including an overload in granite for HTTP::Params
<FromGitter>
<Blacksmoke16> for now
<FromGitter>
<drujensen> to keep this generic, what if we allow for any `Enumerable(K, V)`?
<FromGitter>
<Blacksmoke16> then your validations are tied to your object
<FromGitter>
<Blacksmoke16> which (most of the time?) matches your orm models
<FromGitter>
<drujensen> brb
<FromGitter>
<Blacksmoke16> the main thing im struggling with is how to best support these "extensions", them having their own shard seems overkill, but having them in `Amber` repo makes it hard to keep up to date to Granite changes
<FromGitter>
<Blacksmoke16> having them in `Granite` repo seems like the best idea, but just have them requireable, `require "granite/ext/http"`, which i imagine `Amber` could require automatically?
<FromGitter>
<Blacksmoke16> anyway, we're prob getting ahead of ourselves, ill play around with removing `set_attributes` and see how it goes and go from there
<FromGitter>
<Blacksmoke16> afk driving home
<FromGitter>
<elorest> There still seems to be something in granite which is requiring `crystal-db` 0.7.0. Trying to track that down but I'm in a meeting.
<FromGitter>
<drujensen> ok
<FromGitter>
<elorest> Fixed it. My shard cache had the old tag.
<FromGitter>
<samholst> @drujensen @elorest I'm going to try and get some more of the pending tests to pass on dru's branch