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

Feature: Git Integration Phase 3 #12344

Merged
merged 24 commits into from
May 21, 2023
Merged

Conversation

ferrariofilippo
Copy link
Contributor

@ferrariofilippo ferrariofilippo commented May 12, 2023

Resolved / Related Issues

  • Were these changes approved in an issue or discussion with the project maintainers? In order to prevent extra work, feature requests and changes to the codebase must be approved before the pull request will be reviewed. This prevents extra work for the contributors and maintainers.
    Related Feature: Git integration #11014

IMPORTANT
Please, if you try checking out remotes, place a breakpoint at line 149 of GitHelpers.cs and make sure that the method is executed only once
There was a bug where it would keep on creating new branches because of selection changing. It should be fixed now, but it's better to be careful

Validation
How did you test these changes?

  • Did you build the app and test your changes?
  • Did you check for accessibility? You can use Accessibility Insights for this.
  • Did you remove any strings from the en-us resource file?
    • Did you search the solution to see if the string is still being used?
  • Did you implement any design changes to an existing feature?
    • Was this change approved?
  • Are there any other steps that were used to validate these changes?
    1. Open app and navigate to a git directory
    2. Click on the branch name
    3. Click on New Branch

Screenshots (optional)
2

@ferrariofilippo ferrariofilippo marked this pull request as draft May 12, 2023 16:10
@yaira2 yaira2 marked this pull request as ready for review May 19, 2023 14:18
@hecksmosis
Copy link
Contributor

hecksmosis commented May 19, 2023

I am getting a bug where when you open the branches menu it glitches. (POV: You don't pay for bandicam)

bandicam.2023-05-19.19-38-21-532.mp4

@ferrariofilippo
Copy link
Contributor Author

I am getting a bug where when you open the branches menu it glitches.

Does it eventually stop flickering? That might be related to the changes being applied, and so the directory info being updated. We might fix that using some kind of BulkOperation that we use for other operations

@hecksmosis
Copy link
Contributor

I am getting a bug where when you open the branches menu it glitches.

Does it eventually stop flickering? That might be related to the changes being applied, and so the directory info being updated. We might fix that using some kind of BulkOperation that we use for other operations

Yes, it stops after a bit. Also, this is not as much of a problem in directories with fewer branches

@ferrariofilippo
Copy link
Contributor Author

@hecksmosis can you please test if the last commit fixes the issue?

@hecksmosis
Copy link
Contributor

hecksmosis commented May 19, 2023

@hecksmosis can you please test if the last commit fixes the issue?

I still get the flickering on the latest commit (there are about 40 branches to be shown)

@ferrariofilippo
Copy link
Contributor Author

I can't reproduce the bug, but I know where it may come from, so I'm going trial and error

@hecksmosis
Copy link
Contributor

I can't reproduce the bug, but I know where it may come from, so I'm going trial and error

Ok, ping me and I'll test

@ferrariofilippo
Copy link
Contributor Author

Can you test now, I pushed a commit that should fix it

@hecksmosis
Copy link
Contributor

Yes, it's fixed (btw, it was more like 110 branches so should be robust enough)

Copy link
Member

@yaira2 yaira2 left a comment

Choose a reason for hiding this comment

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

LGTM

@yaira2 yaira2 added ready to merge Pull requests that are approved and ready to merge and removed needs - code review labels May 21, 2023
@yaira2 yaira2 merged commit cf9cdca into files-community:main May 21, 2023
@ferrariofilippo ferrariofilippo deleted the Git_Phase_3 branch May 21, 2023 15:13
Copy link
Member

@0x5bfa 0x5bfa left a comment

Choose a reason for hiding this comment

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

Great work!🎉 Thank you @ferrariofilippo!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge Pull requests that are approved and ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants