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

Update recent commits RFC #1413

Merged
merged 5 commits into from Apr 24, 2018

Conversation

Projects
None yet
2 participants
@simurai
Member

simurai commented Apr 23, 2018

Description of the Change

This moves the "refs" and "un-pulled commits" to the alternatives section. We might reconsider adding them in a log/history view.

Benefits

Makes sure the RFC is accurate, but none of the longer term vision is lost.

Possible Drawbacks

None

Applicable Issues

Part of #1318

@simurai simurai requested a review from smashwilson Apr 23, 2018

@smashwilson

I'd rather not remove things that we still want to do, but just haven't scheduled in our roadmap yet.

To me, this document is a statement of our agreement about the full design of "recent commits" things that we want to do eventually, not just the near-term work that we've happened to implement already. I'd only remove things from the RFC if we've found that we no longer want to do them at all, like ref decoration.

With that said: if there is anything else in here that you'd like to make a case for changing or not doing at all... that would be totally fine 😄

### Refs
Annotate visible commits that correspond to refs in the git repository (branches and tags). If the commit list has been truncated down to ten commits from the full set of relevant commits, display a message below the last commit indicating that additional commits are present but hidden.

This comment has been minimized.

@smashwilson

smashwilson Apr 23, 2018

Member

You made a convincing argument that we shouldn't annotate refs at all. We don't have a lot of room to work with and it wouldn't add that much information... refs are likely best left for the log view.

* Commits reachable by `HEAD` that are not reachable by any local ref in the git repository.
* The single commit at the tip of the branch that was branched from.
The most recent three commits are visible by default and the user can scroll to see up to the most recent ten commits. The user can also drag a handle to resize the recent commits section and show more of the available ten.
The most recent three commits are visible by default and the user can scroll to see up to the most recent ten commits.

This comment has been minimized.

@smashwilson

smashwilson Apr 23, 2018

Member

I think we do still want to make this resizable. Although I will say I find myself not missing it as much as I thought I would.

@@ -42,37 +42,16 @@ Each **recent commit** within the recent commits section summarizes that commit'
* GitHub avatar for both the committer and (if applicable) author. If either do not exist, show a placeholder.
* The commit message (first line of the commit body) elided if it would be too wide.
* A relative timestamp indicating how long ago the commit was created.
* A greyed-out state if the commit is reachable from the remote tracking branch but _not_ from HEAD (meaning, if it has been fetched but not pulled).

This comment has been minimized.

@smashwilson

smashwilson Apr 23, 2018

Member

I'm a little conflicted about this one. Do we still want to do this? I feel like the fetched commits would almost always overwhelm the panel. Maybe we could have a 3 fetched line instead... ?

This comment has been minimized.

@simurai

simurai Apr 24, 2018

Member

I think it's handy to get a hint what the commits are about before you pull the changes. Like when you still have to finish something and can't pull right now, you already know what will come. Since these are on the remote, they could even be click-able and open a browser window with a diff.

That said, I agree that if you're more than 2-3 commits behind, it buries your local changes too much. So I'm ok to punt on it and leave it for a possible log/history.

* A "Revert" option. Choosing this performs a `git revert` on the chosen commit.
* A "Hard reset" option. Choosing this performs a `git reset --hard` which moves `HEAD` and the working copy to the chosen commit. When chosen, display a modal explaining that this action will discard commits and unstaged working directory context. Extra security: If there are unstaged working directory contents, artificially perform a dangling commit, disabling GPG if configured, before enacting the reset. This will record the dangling commit in the reflog for `HEAD` but not the branch itself.
* A "Mixed reset" option. Choosing this performs a `git reset` on the chosen commit.
* A "Soft reset" option. Choosing this performs a `git reset --soft` which moves `HEAD` to the chosen commit and populates the staged changes list with all of the cumulative changes from all commits between the chosen one and the previous `HEAD`.

This comment has been minimized.

@smashwilson

smashwilson Apr 23, 2018

Member

We do want to add these as well... I think we're waiting to have a way to undo them first.

* Navigation button ("open" to a git show-ish pane item)
* Action buttons ("amend" on the most recent commit, "revert", and "reset" with "hard", "mixed", and "soft" suboptions)
![commit-popout](https://user-images.githubusercontent.com/17565/36570682-11545cae-17e8-11e8-80a8-ffcf7328e214.JPG)

This comment has been minimized.

@smashwilson

smashwilson Apr 23, 2018

Member

Definitely still want this. I always want to click on the commits 😄 Figuring out when will be part of our roadmapping this week, I think.

@simurai

This comment has been minimized.

Member

simurai commented Apr 24, 2018

but just haven't scheduled in our roadmap yet

Sorry.. I think I misread the we'll hold off until future quarters as "we might get back to this in a few months". 😅 But the next quarter already starts in a week.

Ok, how about I'll revert everything and only move the "refs" and "fetched but not pulled commits" to alternatives. We can always move the other things later, if we decide not to do them.

simurai added some commits Apr 24, 2018

Revert "Update recent commits RFC"
This reverts commit 503e681.

@smashwilson smashwilson merged commit b54ca06 into master Apr 24, 2018

3 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@smashwilson smashwilson deleted the rfc-recent-commits-edits branch Apr 24, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment