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

HAL support #2

Merged
merged 8 commits into from
Oct 19, 2013
Merged

HAL support #2

merged 8 commits into from
Oct 19, 2013

Conversation

technoweenie
Copy link
Member

Just a quick idea on how we might handle hypermedia parsing. The GitHub API structs would want to extend HALResource into something that checks HAL first, and then *Url properties second.

@owenthereal
Copy link
Member

👍

@technoweenie
Copy link
Member Author

Added support for reflecting hypermedia based on the fields (Url, RepositoryUrl, etc). Most of them will need to use the "rel" annotation so it uses a value consistent with HAL.

As far as the GitHub API goes, we should rely on the reflected hypermedia behavior. Full HAL support would come through a specific media type like "application/vnd.github.hal+json".

Instead of passing the structs around as interface{}, we can pass them around as *HypermediaResource.

rels map[string]Hyperlink
}

func (r *ReflectHypermediaResource) Rels() map[string]Hyperlink {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you planning to move the parsing logic to a decoder?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may need two decoders in the end: json decoder and hyperresource decoder to fill up a HyperResource

@technoweenie
Copy link
Member Author

Okay, this is much easier. I don't care for the names though. Kind of thinking:

sawyer.HypermediaFieldsDecoder
sawyer.HypermediaHALDecoder

It could go into a separate hypermedia package too.

hypermedia.Decoder
hypermedia.HALDecoder

@owenthereal
Copy link
Member

👍 It tastes good. The APIs for this portion start to become elegant!

@technoweenie
Copy link
Member Author

I think I'm going to keep the hypermedia stuff in the sawyer package. The names are longer, but you won't have to import two things every time you use sawyer. What do you think?

@owenthereal
Copy link
Member

I think it makes perfect sense to keep it within the same package for now

@technoweenie
Copy link
Member Author

Cool. Merging this. I don't consider this repo "published" yet, so we can still change names. The README still says "Very experimental", and there aren't really any go docs yet. I'd actually like to successfully integrate this into a couple clients to see how it feels.

technoweenie added a commit that referenced this pull request Oct 19, 2013
@technoweenie technoweenie merged commit 56a17ca into master Oct 19, 2013
@technoweenie technoweenie deleted the hypermedia branch October 19, 2013 22:52
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.

None yet

2 participants