Skip to content
This repository was archived by the owner on Dec 5, 2024. It is now read-only.

File History - #961

Closed
sambonfire wants to merge 12 commits into
github-for-unity:masterfrom
bonfirestudios:master
Closed

sambonfire wants to merge 12 commits into
github-for-unity:masterfrom
bonfirestudios:master

Conversation

@sambonfire

Copy link
Copy Markdown
Contributor

Description of the Change

I'm submitting this PR right now mostly for feedback; there are a handful of things that I'd still like to fix/cleanup; I just want early feedback!

I wanted to be able to display a window showing the history of a single file. On this file history view, users should be able to "check out" an old version of the file. This is a common theme in game development -- just wanting to roll back one file to test vs. all files in a change.

In order to accomplish this, 2 new commands were added to the git client:

  • log history of a file
  • checkout a set of files at a specific change

I also made some interfaces public; this was mostly because I initially started developing the user interface outside of the github plugin because I wasn't sure you'd want it. I've left these changes in this PR because I think that they'll be worthwhile in the future.

To view file history, users can select files in the project file explorer, right click on them, then select Git -> History. This pops up a new window showing the history of the selected file(s).

Alternate Designs

A new view could have been built into the main github window.

Benefits

Users can quickly view history of a single file and revert to any point in history with just a few clicks.

Possible Drawbacks

Currently, there is no checks to see if the file has modifications. As is, the checkout is destructive and this could lead to data loss. I plan on fixing this by adding a confirmation dialog.

Applicable Issues

#948, #947

@StanleyGoldman

Copy link
Copy Markdown
Contributor

I haven't forgotten you. 💛

@StanleyGoldman

Copy link
Copy Markdown
Contributor

I'm working on getting everything that was in master out the door into a release. Then we can start working on this together for the next release. Sorry to keep you hanging.

@sambonfire

sambonfire commented Dec 12, 2018 via email

Copy link
Copy Markdown
Contributor Author

@StanleyGoldman

Copy link
Copy Markdown
Contributor

Happy New Year. This is on my radar this week.

@StanleyGoldman StanleyGoldman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to review the API separately from the UI changes.
It looks good. Also, ApiClient can be made public.

Comment thread src/GitHub.Api/Application/ApiClient.cs
Comment thread src/GitHub.Api/Git/GitClient.cs Outdated
@StanleyGoldman

Copy link
Copy Markdown
Contributor

I sent you a pull request to fix the build

https://github.com/bonfirestudios/Unity/pull/1

@StanleyGoldman

Copy link
Copy Markdown
Contributor

After that pull request is merged in...

Adding some screenshots for further discussion

image

image

@StanleyGoldman

Copy link
Copy Markdown
Contributor

@donokuda put this mockup together for us. I think I like it.

image

So we are going to make a "Pinnable" File Log History view.

@donokuda

Copy link
Copy Markdown
Contributor

👋Adding a little more clarity to what I was thinking with my mock-up. For the most part, everything that is already built is kept as is (including having the file history appear as a separate window.)

The main thing was making the header feel more like the Unity inspector where we show the file's large icon in the header as well as the filename. Clicking the "Show in Project" button would highlight the file in the project pane (which is the current functionality of clicking the "GIT File history for [filepath]" link)

❓Something I'm not 100% sure about is whether it makes sense to show the full path or just the filename. The benefit for the full path is that it ensures that we're looking at the right file and where it's located. The downside is that we can run out of space quickly. Right now, I'm leaning towards showing just the file name since the "Show in Project" button could also be a compromise that's already built.

@sambonfire

Copy link
Copy Markdown
Contributor Author

<3 the mockup and ideas (dockable history view). I should have some time in the next couple weeks to work on this!

sam

History window will prompt to overwrite a file if the local contents
have changed.

Also fixed a couple bugs where the window would cause exceptions after a
domain reload.
@sambonfire

sambonfire commented Jan 19, 2019 •

Copy link
Copy Markdown
Contributor Author

Added some code here that will see if the file has any local changes, and if so it prompts the user to overwrite it (when doing a Checkout on the revision). I also fixed some bugs where the UI was causing exceptions after a domain reload -- it should behave now.

Are there any other outstanding changes you'd like me to make @StanleyGoldman ?

@StanleyGoldman

Copy link
Copy Markdown
Contributor

Hey @sambonfire I'm going to close this pull request, but I'm going to continue to graft whatever remaining functionality you added to the new pull request #999

I don't think you will be able to push to that branch, but you can open additional pull requests adding functionality to that branch.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants