Represent Global IDs as URIs: globalid://basecamp/Todo/1234 #16
Conversation
end | ||
|
||
attr_reader :uri, :app, :model_name, :model_id |
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.
Should we add tests to make sure we are assigning the right URL parts to these instance variables?
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.
Yes. If we're positive on the direction (using URIs) I'll include a real parser as well 😁
(An alternative is to use URNs, but they're wordier and don't have broad generation/parsing support like URIs.) |
@dhh @rafaelfranca what do we do with this? Dhh said to merge globalid into rails in the activejob PR (seuros/actionmailer-deliver_later#6 (comment)) but if we're going to migrate the representation to URIs we should do it ASAP. |
I like the uri idea aswell. We just have to add that to the changelog since it a breaking change. |
Cristian, I’m game for @jeremy’s URI idea. So I say we polish that up as part of this. But @jeremykemper also suggested that we split this out in a separate gem entirely, so Sinatra and others can use it too. I like that idea. So basically just go with a separate “globalid” gem, and then have activejob rely on that. On Aug 12, 2014, at 12:23, Cristian Bica notifications@github.com wrote:
|
I though about the separate gem, but Globalid depend on activemodel. If we embed it , then sinatra app will have just to require activemodel 4.2+ |
What does GlobalID need from activemodel? Could we disconnect it? On Aug 12, 2014, at 12:30, Abdelkader Boudih notifications@github.com wrote:
|
|
We can just have a protocol, imo. That requires that anything that mixes in GlobalID has to provide these things. |
I had implemented something in #10 but it was rejected. |
GlobalID needs class ActiveRecord::Base
global_identificable by: :id, finder: :find
end and with the URI stuff it looks pretty good to me |
I don’t even think we need to make those configurable at this point. We’re just putting out the contract that to use GlobalID, you have to have an #id and a #find method. Then we’ll include GlobalID directly into ActiveModel in the activemodel gem. On Aug 12, 2014, at 13:51, Cristian Bica notifications@github.com wrote:
|
@jeremy can you rebase and merge this PR ? |
@seuros Yep. I have a few more points to complete before merge. |
Actually you're right. When we'll find someone who wants GlobalID and uses id/find for something else we will implement |
Moved to #17 based on the |
Next steps: