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

GitHub web UI: Force push timeline event #24401

Closed
vsemozhetbyt opened this issue Nov 16, 2018 · 5 comments
Closed

GitHub web UI: Force push timeline event #24401

vsemozhetbyt opened this issue Nov 16, 2018 · 5 comments
Labels
meta Issues and PRs related to the general management of the project.

Comments

@vsemozhetbyt
Copy link
Contributor

vsemozhetbyt commented Nov 16, 2018

https://blog.github.com/changelog/2018-11-15-force-push-timeline-event/

Example: timeline event -> diff by force-pushed link

Does this mean that we need not anymore create fixup commits for nit changes in big-diff-PRs to alleviate follow-up reviews?

@vsemozhetbyt vsemozhetbyt added the meta Issues and PRs related to the general management of the project. label Nov 16, 2018
@devsnek
Copy link
Member

devsnek commented Nov 16, 2018

it wasn't ever required to use fixup/squash commits.

also its worth mentioning that at some point in the recent future, github review comments started persisting between force pushes, so it is technically possible to do a pr where you handle the whole review with force pushes.

@vsemozhetbyt
Copy link
Contributor Author

IIRC, I was asked to not force-push addressed comments in big diffs so that reviewers need not rescan all the diff to find what changed.

@refack
Copy link
Contributor

refack commented Nov 16, 2018

image

Does this mean that we need not anymore create fixup commits for nit changes in big-diff-PRs to alleviate follow-up reviews?

Seems like this would solve that issue... Let's see how is the actual implementation affect our workflow.

also its worth mentioning that at some point in the recent future, github review comments started persisting between force pushes

Unless the code possession does not exist anymore (or does not fit some diff algorithm)

IIRC, I was asked to not force-push addressed comments in big diffs so that reviewers need not rescan all the diff to find what changed.

👍

@refack
Copy link
Contributor

refack commented Nov 16, 2018

Another interesting feature - details view of the last commit in a "purple" merged PR.
AFAICT this is visible iff the last commit in the PR is identical to that pushed to the tree, and that commit has tests reported for it.
image

@Trott
Copy link
Member

Trott commented Nov 22, 2018

Feel free to re-open if there's something specific we need to resolve. Otherwise, it seems like this can be closed?

@Trott Trott closed this as completed Nov 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta Issues and PRs related to the general management of the project.
Projects
None yet
Development

No branches or pull requests

4 participants