isaacs / github

Just a place to track issues and feature requests that I have for github
2.21k stars 129 forks source link

Allow commenting on non-pull-request code #284

Open dmugtasimov opened 10 years ago

dmugtasimov commented 10 years ago

When reading someone's code I need to ask an author why he/she code that way, not another or ask for clarfication of piece of code for better understanding or suggest an alternative solution for consideration without making pull request. There fore it would useful communication in the context of the code line, so please, allow commenting on any code in repository.

RSully commented 10 years ago

All code has a commit associated with it, and commits allow commenting (on both the commit, and each line within the commit).

Mithgol commented 10 years ago

While viewing a file, use the “Blame” button to get the commit that introduced the last version of the particular line of the source code.

cirosantilli commented 10 years ago

And if you want something other than commit comments, see also: https://github.com/isaacs/github/issues/211

pb-cdunn commented 8 years ago

+1

maiamcc commented 6 years ago

+1 -- would love a way to comment in a PR on code not changed in the commits associated with that PR. (e.g. sometimes new code has rendered old docstrings obsolete and I want to mention this in a comment.)

holtzermann17 commented 6 years ago

Here's an example using Hypothesis to "inject" comments. I realise this isn't what people were asking for but it might work in a pinch. https://via.hypothes.is/https://gist.github.com/caugner/589cac8dbb07b26b34ff

originalfoo commented 6 years ago

It's a PITA to have to track down a commit relevant to the code (especially if it's from many months ago). Would be nice to be able to comment on a line and have github work out which commit it relates to automagically.

alranel commented 6 years ago

I think what we really want is the ability to start a review attached to an entire file at a given commit, and have it listed/linked among issues.

davidt-secondmind commented 6 years ago

+1 -- Someone creates a PR to show me how they have changed the code to achieve a particular purpose. I think there is some code, unchanged in their PR, that should be changed to achieve the purpose of this PR. I would like to be able to comment on this, and for the PR author, the most useful place for me to leave the comment is next to the (as-yet-unchanged) code. In my mental model, the comment relates to this PR, not to an old commit that made the code how it is.

DevinBirtciel commented 6 years ago

Is anything happening with this?

bordumb commented 5 years ago

+1 This would be really nice to have.

Currently, the only way to add line-by-line comments is to go through a formal pull request workflow. Sometimes you want to allow people to more freely push code (ad-hoc/unimportant work). It doesn't make sense to always force people into making pull requests just so to get comments on code.

Any update on this?

davidstanke commented 5 years ago

I think of this in the context of an audit. Say I have a repo that I've been working on for a while: lots of commits, lots of PRs (or maybe even no PRs, if I'm the sole author). I want to ask a colleague to take a look and give general feedback. Some of their notes may be about project-level design choices, others may be file-level thoughts, and still others may be line-by-line nits. It doesn't matter how the code got to the state it's in; I'm asking for review of current state... the entire current state.

I'm not really sure how to do this. (Other than to create a branch, delete all contents of that branch, then make a PR from the actual repo contents against that blank branch. Which seems rather awkward.)

farrukhnajmi commented 5 years ago

The way GitLab does this is very functional and seammless because it does not require hunting down the commit where code you want to comment was changed. Please consider support similar:

https://docs.gitlab.com/ee/user/project/merge_requests/work_in_progress_merge_requests.html

vadzim-revolist commented 5 years ago

+1 I need to make comments to the code that I think was intended to be changed, but actually wasn't changed. So I need to comment any line of the file, not only near the changes.

aheninger commented 5 years ago

+1 Trying to review a PR that is cleaning up a class of problem in a file, and it missed one occurrence. There's no way to point out the problem where it occurs.

eallenOP commented 5 years ago

Marking student work, I want to be able to leave comments all through any given file on the commit closest to the due time (not just on the lines that changed, because often it's just a merge and nothing changed). I guess I could make an issue for each comment that references the line, but that means a lot of back and forth.

JStickler commented 5 years ago

Same problem with documentation PRs, sometimes changes in the code require updates in the docs. It would be nice to be able to point out, in context, where someone missed something in a documentation update.

krulik commented 5 years ago

What @davidstanke said. This post explains how to do it manually: https://astrofrog.github.io/blog/2013/04/10/how-to-conduct-a-full-code-review-on-github/

arturpat commented 5 years ago

I've just encountered this issue. Anybody found a way to do so? I did what @davidstanke said (delete and re-add) but this is a dirty workaround and it includes using features for totally different purposes than what they were designed for.

barbogast commented 5 years ago

image

😢

molysgaard commented 5 years ago

This is an obvious issue in my company when doing a code-review of a pull-request.

The diff in a PR might require changes to lines that are not changed in the PR. Since these lines are greyed out in a PR-review it is not possible to comment on them.

I guess the underlying problem is that a PR-review is modeled as a review of a diff, instead of a review of the repository state at a certain commit. This is a fundamental flaw in GitHubs design of a review since you can not guarantee that changes in a diff only affect the lines in the diff itself.

A trivial example is renaming a function, but forgetting to rename all usages of it. Why should it not be possible to comment on the call site with the old function name, where the problem actually is located?

arturpat commented 5 years ago

@molysgaard Exactly this! It happens often that the change is affecting the lines that are not part of this commit.

tskopek commented 5 years ago

We just migrated to Github from Bitbucket. I got so used to doing this in Bitbucket, I never considered this would not be possible in Github.

This is an extremely useful feature, for many of the reasons outlined above.

arturpat commented 5 years ago

@tskopek wait is it possible in Bitbucket? I have this issue in Bitbucket. Were you able to work around it?

tskopek commented 5 years ago

Hey @arturpat, well, not exactly in the complete way that this Issue requests (where you can comment on any file in the repo), but in Bitbucket you can at least comment on other parts of the files that are part of the PR, outside of the "diff section". In github I am not even able to do this!

Hopefully this helps explain:

image

arturpat commented 5 years ago

Oh I see what you meant. True, I am able to add comments exactly +/- 10 lines of what was changed. Lines that are further that 10 lines from the changed ones are grayed out and I can't add comments there. Maybe this is something that can be configured in bitbucket config which I don't have access to. Anyway, thank you @tskopek for taking your time to explain :)

TechMaz commented 4 years ago

+1 this, would love the ability to comment on other lines in the code in a pull request

Exr0n commented 4 years ago

+1 this, are there any updates? Is there an official feature request?

neverdieTRX commented 4 years ago

+1 this would be a really useful feature

fbidu commented 4 years ago

Just ran into this! Passing by to give a +1

Markkop commented 4 years ago

This would help a lot on external Code Reviews, please consider this feature :rocket:

markhealy88 commented 4 years ago

Azure DevOps allows commenting on any line of code in a changed file within the context of a pull request. This is a very useful feature, such as highlighting areas that might have been overlooked as part of the aim of the PR

aleksey-rezvov commented 4 years ago

I think I solved a similar task: https://github.com/oktend/system-review-example

BilalIshtiaque commented 4 years ago

This would be very helpful, as others mentioned, a lot of time certain code outside the PR context, esp within the same file, need to be updated as per the PR changes, but being unable to leave comments make it very hard to communicate properly

tanujkhattar commented 4 years ago

Much needed feature! +1

chrisjsewell commented 4 years ago

+1 to bump this up the priority list 😬

calculuschild commented 4 years ago

Another vote for this here. I'm surprised this has been here for more than 5 years already.

denalimarsh commented 4 years ago

+1 bump, please implement this feature.

dubois commented 4 years ago

The current behavior is baked into the design of the github API. When adding a comment, the line is specified relative to the diff being viewed; see this picture from this SO question.

It's too bad, but I think we're stuck with it.

ronakg commented 4 years ago

This is such a basic use-case that comes up so often in our reviews. Someone made a change that applies to multiple places, but they missed a couple. Now there's no way to comment on those 2 places reminding them that they missed.

avanathan commented 4 years ago

Is this 6 years old request 😞 ? Even generic comments (for the entire file) will suffice too, like we have on GitHub issues. Feels like this is basic use case & people use some hacky solutions to get around it. This could be a great feature (for the effort it needs (?)) added to power authors.

krisdale commented 4 years ago

+1 - a much-needed feature!

cgeiershouse commented 4 years ago

+1 - a basic and much-needed feature!

dgw commented 4 years ago

Reviewed a PR today that needs a new Python requirements entry to work, but can't add a suggestion to the relevant file because the PR author didn't touch it.

Not the sexiest example, but suggestions really are more useful if we can actually make them. 😁

muescha commented 4 years ago

yes - sometimes it is more easy to make suggestions than explain which file the PR author has to touch

nitheeshas commented 4 years ago

Just got hit with this limitation. A bit surprised that this use case isn't covered already by GitHub.

KjellKod commented 4 years ago

+1 :

xuto2 commented 4 years ago

+100

jd-lacey commented 4 years ago

Ugh just ran into this issue. This is ridiculous and seems like such an arbitrary restriction. It's been almost 6 years. Come on Github, fix this.

muescha commented 4 years ago

@jd-lacey i think GitHub is not reading this here ... you should send them a feedback