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

Fix #41080 #42003

Merged
merged 1 commit into from
Jan 29, 2018
Merged

Fix #41080 #42003

merged 1 commit into from
Jan 29, 2018

Conversation

KamilSzot
Copy link
Contributor

No description provided.

@joaomoreno joaomoreno added this to the January 2018 milestone Jan 23, 2018
@joaomoreno joaomoreno added git GIT issues extensions Issues concerning extensions and removed git GIT issues labels Jan 23, 2018
@wangpung12
Copy link

Good

@joaomoreno joaomoreno assigned sandy081 and unassigned joaomoreno Jan 26, 2018
@sandy081
Copy link
Member

@KamilSzot It is not mandatory to have a repository, so gallery.assets.repository can be null. Hence I would put null check here similar to others.

@KamilSzot
Copy link
Contributor Author

There's already code nearby that generates repository with empty urls if repo urls can't be found in version.properties: https://github.com/KamilSzot/vscode/blob/e2115a55c22089fa384b332e5b0d8011b587b29f/src/vs/platform/extensionManagement/node/extensionGalleryService.ts#L219

My fix just does the same for the case when there are no version.properties at all.

O course you can just put in the check, that was my first instinct as well. I just wasn't sure if that was the only place in the whole codebase that depended on repository not being null and I saw that in the case when repo url can't be found, the existing code returns object with empty urls rather than null.

@sandy081 sandy081 merged commit 13a3096 into microsoft:master Jan 29, 2018
@sandy081
Copy link
Member

Ok makes sense.

@github-actions github-actions bot locked and limited conversation to collaborators Mar 31, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
extensions Issues concerning extensions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants