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.

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

Did you came out with any solution to add inline comments on a patch? #28

Open
albfan opened this issue Apr 19, 2018 · 5 comments
Open

Comments

@albfan
Copy link

albfan commented Apr 19, 2018

Given the diff output

commit 5234efaf57b4fac96da75eca554da021 (HEAD, origin/master)
Author: albfan <albfan@gnome.org>
Date:   Thu Apr 19 11:43:36 2018 +0000

    My commit

diff --git a/file b/file
index ff249e2..30c8d10 100644
--- a/file
+++ b/file
@@ -4,7 +4,7 @@
        aasdfa
        wertwer
        wetrgsd
-       hello
+       bye
        tryuryt
        rtyurty
        rtyurtuy
diff --git a/foo b/foo
index 1adb588..cd374ea 100644
--- a/foo
+++ b/foo
...
@@ -9,7 +9,7 @@

did you came out with any format to add inline comments?

@albfan albfan changed the title Did you came out with any solution to add inline comments on a patch Did you came out with any solution to add inline comments on a patch? Apr 19, 2018
@shanesmith
Copy link
Owner

Unfortunately not, when I was actively working on this project Gerrit did not provide an API for inline comments... =(

@albfan
Copy link
Author

albfan commented Apr 20, 2018

Does it now? Something like this? https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#fix-replacement-info

If you know the endpoint for it now I can work on it

@shanesmith
Copy link
Owner

That does look promising. =)

There's an issue however in that this tool was written before Gerrit had a REST API (at least the version we had installed at my work), so instead it uses the older "SSH API" documented here: https://review.openstack.org/Documentation/cmd-index.html

If you can find a way to do it in the end you're more than welcome to submit a PR. =)

@albfan
Copy link
Author

albfan commented Apr 23, 2018

Nice. I did similar things in past for old bugzilla CLI, to use its REST API.

Looking at https://www.gerritcodereview.com/releases/README.md I will use gerrit 2.15 to test:

If you know any avaliable gerrit deploy to play with, let me know. I only know eclipse gerrit, but don't think it is avaliable for tests.

So my plan is to parse a modified diff, like so:

commit 5234efaf57b4fac96da75eca554da021 (HEAD, origin/master)
Author: albfan <albfan@gnome.org>
Date:   Thu Apr 19 11:43:36 2018 +0000

    My commit

diff --git a/file b/file
index ff249e2..30c8d10 100644
--- a/file
+++ b/file
@@ -4,7 +4,7 @@
        aasdfa
        wertwer
        wetrgsd
v This is a comment for this hello line
-       hello
+       bye
^ This is a comment for this bye line change
        tryuryt
        rtyurty
        rtyurtuy
diff --git a/foo b/foo
index 1adb588..cd374ea 100644
--- a/foo
+++ b/foo
...
@@ -9,7 +9,7 @@

So ^ means upper line and v means bottom line (line the comment refers too)

@albfan
Copy link
Author

albfan commented May 30, 2018

seems janestreet has been doing this for at least two years janestreet/iron#5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants