Proxy (p)refactor of create tutorial repository dialog#9126
Merged
outofambit merged 22 commits intodevelopmentfrom Feb 19, 2020
Merged
Proxy (p)refactor of create tutorial repository dialog#9126outofambit merged 22 commits intodevelopmentfrom
outofambit merged 22 commits intodevelopmentfrom
Conversation
Technically the resetting isn't necessary but it doesn't cost us anything and both setCreateTutorialRepositoryProps and _closePopup are noops if the popup has already disappeared
outofambit
suggested changes
Feb 18, 2020
Contributor
outofambit
left a comment
There was a problem hiding this comment.
one concern but otherwise looks great!
| import { git } from '../../git' | ||
| import { friendlyEndpointName } from '../../friendly-endpoint-name' | ||
|
|
||
| const nl = __WIN32__ ? '\r\n' : '\n' |
outofambit
approved these changes
Feb 19, 2020
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
As part of the proxy work I'm doing I've tentatively settled on an approach which means that all network Git operations are going to have to originate from within the App Store. The only network Git operation that doesn't originate from the AppStore today is the
pushoperation within theCreateTutorialRepositoryDialogcomponent.In refactoring this I realized that the change was becoming rather large and thus a good candidate to be extracted and reviewed on its own.
Also, @outofambit totes foresaw this happening
Notes for the reviewer
This is in essence a refactor and no functionality should have changed. Evidence of altered logic are probably a mistake worth calling out in the review.
One notable exceptions is that the
CreateTutorialRepositorycomponent used to have a separateloadingstate which was set to true when the create operation started and reset to false once it ended. This state determined whether the dialog would be enabled (i.e. whether the create/cancel buttons could be clicked), whether it was dismissable, and whether it should show the progress spinner or not. This state has been replaced by a variable derived from the presence of a progress property. Functionally to the user there should be no difference.As a side effect of ☝️ I've slightly reordered the
createTutorialRepositorymethod to immediately set a progress upon commencing the creation.