Code review efficiencies

[awindillman]

NOTE: It does not appear I can create an issue with a label other than Bug. I would not classify this as a bug, but rather help wanted (which might change into a bug or feature or features)

Looking at AZD to improve code review quality and efficiency. Some of these are probably easy, others are probably a wish list...

Q: In order to review code with all of the IntelliJ review tools we need to be on the same branch as the PR -- is that correct?

Q: In order to have all the IntelliJ tools we [also] need to "Jump to Source". Is there an option to make that default? Or to get there from the diff (the diff being the default)

Q: Is there a way to "go to next change" when in Review Mode via Jump to Source? (This one would be super, helpful.. arrows like diff would be nice, and a key binding would be great -- otherwise we have to look at the gutters to find the changes)

Q: Is there a way to refresh review data from the timeline view? In general, can the review data be received by "push" from AZD?

[Jonatha1983]

Hi @awindillman

Q: In order to review code with all of the IntelliJ review tools we need to be on the same branch as the PR -- is that correct?

No, this is incorrect. You can review all the PR from the repo you are on. You can't review PR from a different repository, but you should be able to review all the PR in the current repo. If you are on the same branch as the PR, you can navigate in the code evaluated, so if a new function/method was introduced and you want to go see it on your local files, you should be able to. In any case, the changes should be in the PR diff.

In order to have all the IntelliJ tools we [also] need to "Jump to Source". Is there an option to make that default? Or to get there from the diff (the diff being the default)

This is the default - but it is related to the previous answer - if your code is far from what you are reviewing, the jump to the source will not be accurate.

Is there a way to "go to next change" when in Review Mode via Jump to Source? (This one would be super, helpful.. arrows like diff would be nice, and a key binding would be great -- otherwise we have to look at the gutters to find the changes)

Yes, this is a default functionality in Intellij - you can jump to the next and previous files. And you can collapse to see only the changes. See those options:

Q: Is there a way to refresh review data from the timeline view? In general, can the review data be received by "push" from AZD?

There is a refresh both in the timeline and in the diff. The work is the same - it should refresh both the timeline and diff - if they don't, please open a bug ticket. In any case, there is already an open issue with push updates - it is in the backlog.

[Jonatha1983]

Refresh with right-click: and in the diff:

[awindillman]

From: JG9 @.> Sent: Wednesday, February 28, 2024 12:41 PM To: edgafner/dorkag @.> Cc: Neal Dillman @.>; Mention @.> Subject: Re: [edgafner/dorkag] Code review efficiencies (Issue #66)

CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.

Hi @awindillmanhttps://github.com/awindillman

Q: In order to review code with all of the IntelliJ review tools we need to be on the same branch as the PR -- is that correct?

No this is incorrect. You can review all the PR from the repo you are on. You can't review PR from a different repository but you should be able to review all the PR in the current repo. If you are on the same branch as the PR, you can navigate in the code evaluated, so if a new function/method was introduced and you want to go see it on your local files, you should be able to. In any case, the changes should be in the PR diff.

[Neal Dillman] I was unable to “Jump to Source” (as on the whole file not the diff mode) unless I was on the same branch. I’ll do some more testing with the next few pushes and see if I can see what the factors might be.

In order to have all the IntelliJ tools we [also] need to "Jump to Source". Is there an option to make that default? Or to get there from the diff (the diff being the default) This is the default - but it is related to the previous answer - if your code is far from what you are reviewing, the jump to the source will not be accurate.

[Neal Dillman] The default appears to take you to the diff. @.*** I have to choose Jump to Source or F4 to get to the single file in review mode.

Is there a way to "go to next change" when in Review Mode via Jump to Source? (This one would be super, helpful.. arrows like diff would be nice, and a key binding would be great -- otherwise we have to look at the gutters to find the changes) Yes, this is a default functionality in Intellij - you can jump to the next and previous files. And you can collapse to see only the changes. See those options: [Neal Dillman] That picture is in diff mode. I need to see it in single file mode. It should have something like this in the upper right in order to get all of the “code intelligence” features I am looking to take advantage of: @.***

[Neal Dillman] In that mode I don’t have a way to jump to the next/previous change and I can only use the gutters.. I think.

Q: Is there a way to refresh review data from the timeline view? In general, can the review data be received by "push" from AZD? There is a refresh both in the timeline and in the diff.

[Neal Dillman] I could not find a refresh button on either, but I did find that there is a right click option to do that.. so that works for now.

The work is the same - it should refresh both the timeline and diff - if they don't, please open a bug ticket. In any case, there is already an open issue with push updates - it is in the backlog.

[Neal Dillman] Excellent. Thank you. I’ll see if I can find it to vote for it.

— Reply to this email directly, view it on GitHubhttps://github.com/edgafner/dorkag/issues/66#issuecomment-1969881396, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AF7O3K4LM73DH4CM6SMX3DDYV6I4HAVCNFSM6AAAAABD6WTIJWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNRZHA4DCMZZGY. You are receiving this because you were mentioned.Message ID: @.***>

[Jonatha1983]

@awindillman I think you copied an entire email or so. Let me know if we can close this ticket for now.

[awindillman]

No this is incorrect. You can review all the PR from the repo you are on. You can't review PR from a different repository but you should be able to review all the PR in the current repo. If you are on the same branch as the PR, you can navigate in the code evaluated, so if a new function/method was introduced and you want to go see it on your local files, you should be able to. In any case, the changes should be in the PR diff.

[Neal Dillman] I was unable to “Jump to Source” (as on the whole file not the diff mode) unless I was on the same branch. I’ll do some more testing with the next few pushes and see if I can see what the factors might be.

[Neal Dillman] The default appears to take you to the diff. I have to choose Jump to Source or F4 to get to the single file in review mode.

Yes, this is a default functionality in Intellij - you can jump to the next and previous files. And you can collapse to see only the changes. [Neal Dillman] That picture is in diff mode. I want to see it in single file mode. It should have something like this in the upper right in order to get all of the “code intelligence” features I am looking to take advantage of:
[Neal Dillman] In that mode I don’t have a way to jump to the next/previous change and I can only use the gutters.. I think. Next/previous is present in the diff, just not the single file. There is a macro for next/previous change, but it will only work if I make a local change (in other words, it is not considering changes from the PR when it jumps)

Q: Is there a way to refresh review data from the timeline view? In general, can the review data be received by "push" from AZD? There is a refresh both in the timeline and in the diff. [Neal Dillman] I could not find a refresh button on either, but I did find that there is a right click option to do that.. so that works for now.

The work is the same - it should refresh both the timeline and diff - if they don't, please open a bug ticket. In any case, there is already an open issue with push updates - it is in the backlog. [Neal Dillman] Excellent. Thank you. I’ll see if I can find it to vote for it.

[Jonatha1983]

@awindillman

[Neal Dillman] I was unable to “Jump to Source” (as on the whole file not the diff mode) unless I was on the same branch. I’ll do some more testing with the next few pushes and see if I can see what the factors might be.

If the file is a new file introduced by the PR, you can only see it locally if you are checkout to that PR's branch. There is an existing bug in the IDE that the "Jump to Source" is not working correctly, but this is not related to the plugin.

That picture is in diff mode. I want to see it in single-file mode. It should have something like this in the upper right in order to get all of the “code intelligence” features I am looking to take advantage of:

Yes, that is right; to benefit from the IDE problem analysis, you must check the PR branch and go to the file. There is no Builtin Analysis inspection on the diff files.

[Jonatha1983]

You can jump to the source even if you are on a different branch than the PR - but only in case the source exists.

https://github.com/edgafner/dorkag/assets/3816566/b9bca8e4-c23d-46a9-8e77-fb5d3d2eca21

I also had some issues with the jump to source when using the general IDE diff - but again probably a bug not related to the Plugin. The functionality should work.

[awindillman]

That picture is in diff mode. I want to see it in single-file mode. It should have something like this in the upper right in order to get all of the “code intelligence” features I am looking to take advantage of:

Yes, that is right; to benefit from the IDE problem analysis, you must check the PR branch and go to the file. There is no Builtin Analysis inspection on the diff files.

OK. That is consistent with what I am seeing.

[awindillman]

Once we are in review mode: is there a way to jump from one changed section to the next? Looking for the same functionality as exists with diff. I noted that the editor supports it.. but it only sees locally made changes.

[Jonatha1983]

In the upcoming ide version 241 they added this functionality and it will be available for the plugin users as well

[awindillman]

Looks like AZD doesn't work with 241 EAP yet?

[Jonatha1983]

No I am releasing tomorrow a version in eap channel

[awindillman]

I am closing this as it seems like the items are all covered in one way or another. From a priority perspective push of changes (like comment responses) would be the last big request once the EAP comes out. I'm also putting in a ticket to request adding mentions.

[awindillman]

No I am releasing tomorrow a version in eap channel

I just downloaded it. After some clicking around I found the new feature. Works great. Thank you!

[Jonatha1983]

@awindillman Thanks for updating.

I would really appreciate if you could give the plugin a review in the marketplace

Have a great weekend 🤙🏻