search

AGhost-7 / critiq.vim

Github code reviews from Neovim
73 stars 4 forks source link

[Enhancement] Bitbucket #18

Open kristijanhusak opened 5 years ago

kristijanhusak commented 5 years ago

Hi,

any plans for supporting bitbucket?

AGhost-7 commented 5 years ago

Not planned, but I did want to support other services at some point. Does bitbucket have feature parity with github?

kristijanhusak commented 5 years ago

Yeah, it does. Only two things are missing if im not mistaken:

Labels Request PR changes action.

Those could be just skipped.

AGhost-7 commented 5 years ago

I'll try to refactor so that new "providers" can be implemented as a first step.

AGhost-7 commented 5 years ago

Just completed the refactor, should be much easier to add support for other services now. There's still some work to be done for new services to be added, but most of the code has been layered in a way that it should largely just be adding a new file in the providers directory and implementing the same handlers as the github one.

kristijanhusak commented 5 years ago

Ok thanks, i'll try to implement bitbucket provider.

kristijanhusak commented 5 years ago

I ran into the first issue. Github api returns formatted json, where bitbucket returns it raw. Since bitbucket doesn't support returning json in that format, i'm trying to make github return raw json. I tried using this accept header application/vnd.github.VERSION.raw+json, but unfortunately curl still returns formatted json. Do you have any idea how could i make this work?

AGhost-7 commented 5 years ago

The formatting shouldn't cause any issues AFAIK. What error did you encounter with parsing JSON from bitbucket? The critiq#request#send function might need to be adjusted.

kristijanhusak commented 5 years ago

yeah that function needs to be tweaked a bit to make this work, because bitbucket returns single entry in stdout, which sets the body to empty dictionary. Why it does that (send function)?

AGhost-7 commented 5 years ago

Hey, I think I fixed the issue. I tested my fix against the JIRA API and it seems to work now. Here's the relevant commit: https://github.com/AGhost-7/critiq.vim/commit/a96bf624479eedccdbdcf1803f116c6ae2d28734

Its merged into master so all you should have to do is pull in my changes.

The issue seems to have that the body wasn't terminated with a line break. Since the last line is supposed to be the http status code this was making the whole body disappear.

AGhost-7 commented 5 years ago

@kristijanhusak,just wanted to mention that I've added a generic options parser in commit https://github.com/AGhost-7/critiq.vim/commit/43890ba32e11af17a2212b03decab387f503bbf6 for when you start up the plugin. I was thinking maybe we could use this to determine the provider, e.g.: :Critiq -provider=bitbucket neovim/neovim. Thoughts?

kristijanhusak commented 5 years ago

I started it, but didn't had time to continue working on it. We could determine the provider by the remote url (git config --get remote.origin.url)

ptn commented 4 years ago

This looks interesting! @kristijanhusak I know it's been a long time, but any chance you still have your code somewhere? Even a wip commit would be helpful.

kristijanhusak commented 4 years ago

@ptn unfortunately not. I had only like 20 lines of code that i ditched at the end. Don't think it would work now, it's been a long time.