dingoblog / dingo

Blog engine written in Go
MIT License
284 stars 37 forks source link

partial post api #71

Closed samdfonseca closed 8 years ago

samdfonseca commented 8 years ago

I've been working on the API starting with post endpoints and I'm finally at a point where I think its worth getting some feedback, since I'm making some non-trivial changes. This is still a work in progress, but you should be able to build the pr branch and see how the changes are looking. I plan on reorganizing the app/handler package into 3 new packages, app/handler/blog for the main public blog pages, app/handler/api for the api stuff, and app/handler/admin for the admin pages, but that will prolly be in a separate pr.

Some high level bullets

dinever commented 8 years ago

Hi @samdfonseca,

I think this is going to the right direction. The challenge is that we may need to deal with some cyclic import problems after we split up the handlers, as those tests require to import handler/init.go to create app mock-ups to test the request handling. However, we will see how to deal with that when we get there.

For the RESTful APIs, I think you're right and we should go with /api/posts/:post_id/comments.

Thanks!

samdfonseca commented 8 years ago

@bentranter let me know if you're still interested in working together on this. if you have a spare few minutes, i'd love some feedback on the changes here.

bentranter commented 8 years ago

Hey @samdfonseca, I'm definitely still interested in working together on this. I looked over this PR for a while tonight and I think everything looks good so far, but I'll give it a more thorough review first thing tomorrow and leave some comments. I really like your idea/comment about following REST guidelines for naming, I feel like sensible naming and URL paths can make or break an API, and this definitely makes it. Awesome decision.

bentranter commented 8 years ago

@samdfonseca I left a few nit-picky comments, feel free to ignore any of them if you don't agree with them since some are just opinion based. Overall though, wow, this looks really, really good. I think this approach and design totally works for this API, and I'm glad everything is being split into its own package.

samdfonseca commented 8 years ago

hey guys sorry i left this hanging. i just merged the changes in master for the last few weeks back into here and didn't run into any major conflicts. unless someone tells me not to, i'm gonna go ahead and merge it in since the none of this api stuff is being used currently