Extract IncludeTree.

[beauby]

Purpose

Extract the IncludeTree class (new source) into its own gem (jsonapi, in an attempt to share some work with jsonapi-resources (c.f. cerebris/jsonapi-resources#679).

Changes

Caveats

Related GitHub issues

Additional helpful information

[NullVoxPopuli]

doh, now it needs rebase :-(

[beauby]

No worries, should be an easy one. Plus, let's wait for the gem renaming.

[NullVoxPopuli]

Removing 'Needs Team Discussion' because we already discussed what to do with this.
To kinda summarize:

• rename the gem to help out with cliry (pending mr. minecraft gives permission / frees up the jsonapi gem namespace)

[beauby]

Unrelated to this PR but this method could be rewritten as follows:

def resource_relationships(options)
  serializer.associations(@include_directive)
            .each_with_object({}) do |association, hash|
    hash[association.key] = relationship_value_for(association, options)
  end
end

[beauby]

Rebased and changed the gem's name to jsonapi as well as the namespace to JSONAPI.

[NullVoxPopuli]

💯

[NullVoxPopuli]

LGTM, just two more things before merging:

• document the new option in the docs (include_directive)
• Changelog Entry

[bf4]

Yay! though I don't think the jsonapi gem should be implementing the * since that's not part of the spec and makes valid params checking a bit weird :)

[beauby]

@bf4 The default behavior of the gem is to ignore *. One can enable it via the "experimental option" allow_wildcards. Moreover, there is talk about adding some kind of wildcard in the spec. I mainly added it not to break backwards compatibility in AMS, but I'm totally open to discussing removing it.

[bf4]

Awesome response!

B mobile phone

On May 16, 2016, at 2:56 PM, Lucas Hosseini [email protected] wrote:

@bf4 The default behavior of the gem is to ignore *. One can enable it via the "experimental option" allow_wildcards. Moreover, there is talk about adding some kind of wildcard in the spec. I mainly added it not to break backwards compatibility in AMS, but I'm totally open to discussing removing it.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub

[NullVoxPopuli]

needs rebase :-(

[beauby]

Rebased and changelog added!

[beauby]

Rebased again. We should probably pin the dependency to jsonapi though, as it is in beta.

[NullVoxPopuli]

@beauby one last rebase!?

[beauby]

Rebased 👍

[NullVoxPopuli]

LGTM! good work @beauby. Your JSON API gem is great! :-)