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

Ask to add known huge folders to .gitignore, fixes #44304 #44562

Merged
merged 4 commits into from Dec 20, 2018

Conversation

Projects
None yet
3 participants
@Mrigank11
Copy link
Contributor

Mrigank11 commented Feb 27, 2018

fixes #44304

Mrigank11 added some commits Feb 27, 2018

@msftclas

This comment has been minimized.

Copy link

msftclas commented Feb 27, 2018

CLA assistant check
All CLA requirements met.

@joaomoreno joaomoreno added this to the Backlog milestone Feb 28, 2018

@joaomoreno joaomoreno added the git label Feb 28, 2018

@joaomoreno
Copy link
Member

joaomoreno left a comment

There's a lot about this which is suboptimal. The user should know which folders would be added to gitignore. This doesn't check whether node_modules is already in gitignore at all. This should also be done before any large git status even runs; right now, the Repository will still get into a isRepositoryHuge = true state... the user would still have to reload to get out of it, even after adding to gitignore.

@Mrigank11

This comment has been minimized.

Copy link
Contributor Author

Mrigank11 commented Jun 4, 2018

@joaomoreno Thanks for the review. I tried to solve the issues you mentioned:

The user should know which folders would be added to gitignore.

done

This doesn't check whether node_modules is already in gitignore at all.

I used the existing checkIgnore method to do that.

This should also be done before any large git status even runs; right now, the Repository will still get into a isRepositoryHuge = true state... the user would still have to reload to get out of it, even after adding to gitignore.

This was solved by saving the .gitignore the in the ignore method because updateModelState was going to be called anyway. So, the repository will be rechecked.

@joaomoreno joaomoreno closed this Sep 11, 2018

@joaomoreno joaomoreno reopened this Sep 11, 2018

@joaomoreno

This comment has been minimized.

Copy link
Member

joaomoreno commented Sep 11, 2018

This was solved by saving the .gitignore the in the ignore method because updateModelState was going to be called anyway. So, the repository will be rechecked.

This is not true. The repository will be in a isRepositoryHuge state, which force the user to reload the window. Give it a try.

@Mrigank11

This comment has been minimized.

Copy link
Contributor Author

Mrigank11 commented Sep 18, 2018

If the user presses 'Yes' to the prompt, .gitignore will be saved and updateModelState will be called, so the user won't have to reload the window as updateModelState will update the repo state i.e. isRepositoryHuge will be calculated again (#1).

So, the only case I'm missing is 'when the user modifies .gitignore manually' (right?). How about we use onFSChange to monitor .gitignore (only when isRepositoryHuge is true) and call updateModelState if it changes?

@joaomoreno joaomoreno merged commit d1ba7ad into Microsoft:master Dec 20, 2018

1 of 3 checks passed

VS Code #20180911.45 failed
Details
VSTS: VS Code 20180604.46 failed
Details
license/cla All CLA requirements met.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment