Skip to content
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

Deprecate gid/sgid/global_id/signed_global_id #36

Closed
wants to merge 1 commit into from

Conversation

chancancode
Copy link
Member

Did a quick spike to see how we can gracefully handle the rename, but it took more code than I thought, so I am not sure if it's worth the effort. However I suppose we will cut a 1.0 release to go out with 4.2.0.final, so not like this code is staying for a long time anyway.

Feel free to reject this and just rename it if you don't think it's worth the trouble. This is still < 1.0 so I suppose it's fair game either way.

Closes #34

Use `to_gid`/`to_sgid`/`to_global_id`/`to_signed_global_id` instead.
@rafaelfranca
Copy link
Member

Do we need to deprecate? I mean, we are in beta...

@chancancode
Copy link
Member Author

I wanted to reward the earlier adopters by making their life's easier, but like I said this is more code than I expected, so I am indifferent 😄

@dhh
Copy link
Member

dhh commented Aug 26, 2014

IMO no need to deprecate. Let's just go straight to the new method names.

On Aug 27, 2014, at 5:15, Rafael Mendonça França notifications@github.com wrote:

Do we need to deprecate? I mean, we are in beta...


Reply to this email directly or view it on GitHub.

@chancancode
Copy link
Member Author

👍 feel free to pick this up, I won't be able to resubmit this today —
Sent from Mailbox

On Tue, Aug 26, 2014 at 11:05 AM, David Heinemeier Hansson
notifications@github.com wrote:

IMO no need to deprecate. Let's just go straight to the new method names.

On Aug 27, 2014, at 5:15, Rafael Mendonça França notifications@github.com wrote:

Do we need to deprecate? I mean, we are in beta...

Reply to this email directly or view it on GitHub.

Reply to this email directly or view it on GitHub:
#36 (comment)

@jeremy
Copy link
Member

jeremy commented Aug 26, 2014

I like that #to_* resolves the collisions, but it evokes the Ruby convention of type coercion. It'd be like asking record.to_class or record.to_id. These are identifying characteristics of the record, not serialized representations of it.

A Global ID is a unique identifier for a record, not a serialization of the record itself. We get a reference to a record, serialize it in Active Job params, then deserialize Active Job params and dereference the Global ID to retrieve the record.

Some options:

  • post.globalid
  • post.globalid_uri
  • post.global_uri
  • post.gid_uri
  • post.global_ref
  • post.gref
  • GlobalID(post)

@dhh
Copy link
Member

dhh commented Aug 26, 2014

I don’t know. It seems pretty similar to stuff like #to_param as well. The GlobalID is not an inherent attribute of the object, like #id or #class. It’s a conversion. Similar to #to_json as well. I think there’s enough precedence there for it to be a good fit. Not liking any of the other options doesn’t help either.

On Aug 27, 2014, at 6:41 AM, Jeremy Kemper notifications@github.com wrote:

I like that #to_* resolves the collisions, but it evokes the Ruby convention of type coercion. It'd be like asking record.to_class or record.to_id. These are identifying characteristics of the record, not serialized representations of it.

A Global ID is a unique identifier for a record, not a serialization of the record itself. We get a reference to a record, serialize it in Active Job params, then deserialize Active Job params and dereference the Global ID to retrieve the record.

Some options:

post.globalid
post.globalid_uri
post.global_uri
post.gid_uri
post.global_ref
post.gref
GlobalID(post)

Reply to this email directly or view it on GitHub.

@kaspth
Copy link
Contributor

kaspth commented Sep 10, 2014

Fixed in 8f893e1

@kaspth kaspth closed this Sep 10, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

alias gid group_id
5 participants