-
Notifications
You must be signed in to change notification settings - Fork 58
Error in HasBinaryUuid trait...? #51
Comments
Of course, I've just noticed that I'm not sure if the docs are wrong, the code is wrong, or I'm wrong in how I've understood things. It seems like the docs allow for a uuid primary key with any name, but the code doesn't. For what it's worth, I prefer the flexibility reflected in the docs and, without being too familiar with the package or its various features yet, would prefer a solution that allows one to craft the solution one wants for individual models. That would include the following:
I'm sure there's more, but like I said, I've not yet begun to use this package...much. ;-) Thanks for your time. :-) |
@brendt i don't think it has anything to do with #49 . @telkins you can modify the what |
@telkins from what you said, you haven't started using this package so there isn't an actual error case here or am i wrong ? In all, i think the README isn't explanatory enough. I can volunteer later on to include better customization examples in the README maybe later this month (if that's okay with @brendt ) via a fresh PR. |
@isocroft Thanks for the info/help. I'm not sure why, but I didn't even consider overriding the No, I really haven't begun to use it just yet. I'm in the early stages of a new project. I'm wanting to use laravel-event-projector and then this package for (binary) UUIDs. I was also hoping to use Nova for administration, but I'm a little wary about how nicely Nova and an event sourced system will play together. So....things have stalled a bit while I wait for Nova and I'm now focusing a bit more on some other things. So...I'd consider this a non-issue now and I'll close it. Your help's been appreciated. :-) |
This is the commit that broke the documented functionality. By deprecating the I think overriding two methods on each model using a binary UUID is annoying and would prefer the old functionality. I'm curious as to why the deprecation was done. I can't find a related PR so it's hard to tell... @brendt could you provide some insight into your commit? |
This is the related issue: #34 Unfortunately, I don't have the time this and next week to look further into this 😞However, feel free to suggest a solution! |
@brendt I think I'd need to understand why |
Isn't this method in
Spatie\BinaryUuid\HasBinaryUuid
likely to cause errors when explicitly setting the primary key...?The examples seem to allow for explicitly setting the primary key to something other than
uuid
:The text was updated successfully, but these errors were encountered: