search

innoq / statuses

statuses
Apache License 2.0
13 stars 14 forks source link

Render avatar, profile and self in format json #166

Closed echox closed 9 years ago

echox commented 9 years ago

Fix for Issue #164 (added self and avatar to json)

mvitz commented 9 years ago

Thanks for your contribution.

  1. I think the self link should point to the 'status' and not to the 'author'!
  2. I personally favour that each commit is buildable. Maybe you could squash the first two and the last two together?
  3. Maybe you could add some tests for the JSON rendering.

I'm going to merge this PR once at least 1. is fixed.

echox commented 9 years ago

Ok:

  1. You are correct, I still keep the profile link as an addition :-)
  2. done.
  3. done, a test for routes.clj should be also added at some point.
FND commented 9 years ago

I personally favour that each commit is buildable

FWIW, I think it's more important for commits to be atomic. However, it's nice if the set of commits being pushed (hopefully still reasonably small) is CI compliant.

mvitz commented 9 years ago

I agree. But for me atomic includes that the code works...

I never would commit a call to a new function first and the function itself in a separate commit afterwards.

mvitz commented 9 years ago

@echox is this PR complete or is there anything missing?

echox commented 9 years ago

should be complete :-)

mvitz commented 9 years ago

Thanks, I will have a look at it at the weekend.