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

Ability to add issues to "actual log" of a review branch #56

Open
jadahl opened this issue Dec 5, 2013 · 2 comments
Open

Ability to add issues to "actual log" of a review branch #56

jadahl opened this issue Dec 5, 2013 · 2 comments

Comments

@jadahl
Copy link

jadahl commented Dec 5, 2013

Whenever a merge or a rebase happens in a review, it becomes less convenient to review the changes as a whole (i.e. selecting all of the fixups and looking at the result).

The way to do it AFAIK is to rebase the review and then look at the "actual log", but in this view there is no way of adding new issues as it is just a plain git log view.

Therefore, it would be nice to be able to review the whole branch, with the ability to view and add issues, in an "actual log" kind of view.

@jensl
Copy link
Owner

jensl commented Dec 5, 2013

Yes, this is an issue. You can usually always use the [reviewable], [relevant] or [full] filters to look at all changes in the review (filtered in different ways to only show some of the modified files.) In the view they produce, you effectively look at the diff between the current tip of the review branch and the "upstream commit," but you can still mark changes as reviewed, which marks them as reviewed in the original commits in the review.

It doesn't work in some cases, but I think that's mostly when you push irregular merges to the review (merging in a commit that is not a descendant of the review branch's "upstream commit.")

The reason you can't mark changes as reviewed when you look at commits in the "actual log" is that it's not easy to map that back to the original commits. It works in the [reviewable], [relevant] and [full] filter cases because they show all changes, and mapping "all rebased changes" to "all original changes" is safe.

One way to avoid it would be to let the rebased commits replace the original commits in the review when the review is rebased, but that becomes really tricky if the original commits are partially reviewed, since there may not be any obvious way to translate that partial state onto the new commits.

@jadahl
Copy link
Author

jadahl commented Dec 5, 2013

I didn't know about theh [reviewable] etc, which covers most of the use cases I was looking for.

Maybe some other of the use cases I had in mind (being able to select commits over a rebase) could be solved similar to [reviewable] but only for the selected commits (or their corresponding rebased commit).

Anyhow, you can consider this a low priority as far as I'm concerned, as [reviewable] addresses most of my use cases I was looking for.

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