HacktheUniverse / star-api

All these stars belong to you
http://star-api.herokuapp.com/
80 stars 16 forks source link

Cannot show stars/1 #22

Open zniazi opened 9 years ago

zniazi commented 9 years ago

I pushed code near the end of Saturday that fixes this and shows /stars/1 or constellations/1 etc. Please check that push.

surenm commented 9 years ago

@zniazi Can you please raise a pull request with the appropriate commits?

zniazi commented 9 years ago

Yes, will submit it later

Sent from my iPhone

On Nov 10, 2014, at 2:41 PM, Surendran Mahendran notifications@github.com wrote:

@zniazi Can you please raise a pull request with the appropriate commits?

— Reply to this email directly or view it on GitHub.

nicholalexander commented 9 years ago

@zniazi what does stars/1 mean? we are currently only referencing the stars resource by the label. what is 1 in this context?

zniazi commented 9 years ago

Oh I see, I meant to reference them by their id. I feel like developers are used to that. Especially as these stars have many associations to other objects.

Sent from my iPhone

On Nov 12, 2014, at 12:01 PM, nichol alexander notifications@github.com wrote:

@zniazi what does stars/1 mean? we are currently only referencing the stars resource by the label. what is 1 in this context?

— Reply to this email directly or view it on GitHub.

nicholalexander commented 9 years ago

interesting - i think the record id is a dangerous thing to correlate to the star resource because the id could be something different depending on the database used and it is not nessecarily the identifying element of a given resource. perhaps with constellations it is different but i think stars should definitely be referenced by their label... maybe.

surenm commented 9 years ago

In some senses the label is the id here. We need to add unique key to the label in the database.

On 12-Nov-2014, at 1:37 pm, nichol alexander notifications@github.com wrote:

interesting - i think the record id is a dangerous thing to correlate to the star resource because the id could be something different depending on the database used and it is not nessecarily the identifying element of a given resource. perhaps with constellations it is different but i think stars should definitely be referenced by their label... maybe.

— Reply to this email directly or view it on GitHub.

nicholalexander commented 9 years ago

right, the label is the id - that's what i was going for. the uniqueness on the label is just doing an add_index on the label column, yes?

On Wed, Nov 12, 2014 at 1:41 PM, Surendran Mahendran < notifications@github.com> wrote:

In some senses the label is the id here. We need to add unique key to the label in the database.

On 12-Nov-2014, at 1:37 pm, nichol alexander notifications@github.com wrote:

interesting - i think the record id is a dangerous thing to correlate to the star resource because the id could be something different depending on the database used and it is not nessecarily the identifying element of a given resource. perhaps with constellations it is different but i think stars should definitely be referenced by their label... maybe.

Reply to this email directly or view it on GitHub.

Reply to this email directly or view it on GitHub https://github.com/HacktheUniverse/star-api/issues/22#issuecomment-62768672 .

zniazi commented 9 years ago

Sounds good to me. The label if unique can serve as the id. One thing to consider down the road is that it may get annoying for developers who work with specific stars to reference them by long labels. But we should move forward by making labels unique and updating the documentation so that that is clear to developers.

On Wed, Nov 12, 2014 at 1:47 PM, nichol alexander notifications@github.com wrote:

right, the label is the id - that's what i was going for. the uniqueness on the label is just doing an add_index on the label column, yes?

On Wed, Nov 12, 2014 at 1:41 PM, Surendran Mahendran < notifications@github.com> wrote:

In some senses the label is the id here. We need to add unique key to the label in the database.

On 12-Nov-2014, at 1:37 pm, nichol alexander <notifications@github.com

wrote:

interesting - i think the record id is a dangerous thing to correlate to the star resource because the id could be something different depending on the database used and it is not nessecarily the identifying element of a given resource. perhaps with constellations it is different but i think stars should definitely be referenced by their label... maybe.

Reply to this email directly or view it on GitHub.

Reply to this email directly or view it on GitHub < https://github.com/HacktheUniverse/star-api/issues/22#issuecomment-62768672

.

— Reply to this email directly or view it on GitHub https://github.com/HacktheUniverse/star-api/issues/22#issuecomment-62769639 .

nicholalexander commented 9 years ago

cool!

On Wed, Nov 12, 2014 at 2:09 PM, Zak Niazi notifications@github.com wrote:

Sounds good to me. The label if unique can serve as the id. One thing to consider down the road is that it may get annoying for developers who work with specific stars to reference them by long labels. But we should move forward by making labels unique and updating the documentation so that that is clear to developers.

On Wed, Nov 12, 2014 at 1:47 PM, nichol alexander < notifications@github.com> wrote:

right, the label is the id - that's what i was going for. the uniqueness on the label is just doing an add_index on the label column, yes?

On Wed, Nov 12, 2014 at 1:41 PM, Surendran Mahendran < notifications@github.com> wrote:

In some senses the label is the id here. We need to add unique key to the label in the database.

On 12-Nov-2014, at 1:37 pm, nichol alexander < notifications@github.com

wrote:

interesting - i think the record id is a dangerous thing to correlate to the star resource because the id could be something different depending on the database used and it is not nessecarily the identifying element of a given resource. perhaps with constellations it is different but i think stars should definitely be referenced by their label... maybe.

Reply to this email directly or view it on GitHub.

Reply to this email directly or view it on GitHub <

https://github.com/HacktheUniverse/star-api/issues/22#issuecomment-62768672

.

Reply to this email directly or view it on GitHub < https://github.com/HacktheUniverse/star-api/issues/22#issuecomment-62769639>

.

Reply to this email directly or view it on GitHub https://github.com/HacktheUniverse/star-api/issues/22#issuecomment-62774859 .