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.

Sign up for GitHub

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

Jump to bottom

What do you think about allowing the server to force a link to be an array #7

Open
chadkouse opened this issue Mar 28, 2014 · 5 comments · May be fixed by #8
Open

What do you think about allowing the server to force a link to be an array #7

chadkouse opened this issue Mar 28, 2014 · 5 comments · May be fixed by #8

Comments

@chadkouse
Copy link

For example - when you have a link relation that is sometimes an array, and sometimes a single link - it can confuse clients to have to figure out when it is and when it isn't. I have built the code to support a 3rd argument to the Resource.link method which is an optional boolean value to indicate forcing an array. If you think it's useful I'll send a pull request.

Here's my work: chadkouse@8e206c3

Thanks
@jpbochi
Copy link
Owner

jpbochi commented Mar 28, 2014

Looks interesting.

However, I'm not a fond of receiving boolean parameters for this kind of thing. On something like resource.link('employee', '/one', true) the true there doesn't give you any clue about its meaning.

How about, for example, resource.link('employee', ['/one'])? It is more explicit about the array, and it would also be a short-cut for adding several links with the same relation.

Since you have a fork of my repo, you can implement it there and pretty easily created a pull request with the changes from your repo to mine. If you do that, naturally, add some unit tests to it.

@chadkouse chadkouse linked a pull request Mar 28, 2014 that will close this issue
Open
@chadkouse
Copy link
Author

Now that I'm looking at it again, maybe if you do resource.link('employee', ['/one', '/two']) it should add 2 links with the same rel?

@chadkouse
Copy link
Author

I updated the PR to do that ☝️

@chadkouse
Copy link
Author

(just realized that's what you were talking about in your initial comment...)

@chadkouse
Copy link
Author

I updated #8 and tightened / cleaned it up - see what you guys think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Force a single link to an array if requested chadkouse/halberd
2 participants
@jpbochi @chadkouse