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

Allow packaging extension without a publisher #1040

Merged
merged 4 commits into from
Aug 23, 2024
Merged

Conversation

benibenj
Copy link
Contributor

@benibenj benibenj commented Aug 22, 2024

This pull request includes changes to the validation of the publisher field in the package manifest. Previously, the validation was triggered even if no publisher was set. With this change, the validation is only performed if a publisher is specified, allowing users to package an extension without a publisher for testing purposes.

fixes #1029

aeschli
aeschli previously approved these changes Aug 22, 2024
Tyriar
Tyriar previously requested changes Aug 22, 2024
src/package.ts Outdated
Comment on lines 1271 to 1275

// allow users to package an extension without a publisher for testing reasons
if (manifest.publisher) {
validatePublisher(manifest.publisher);
}
Copy link
Member

Choose a reason for hiding this comment

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

Something seems off with the types here? If manifest.publisher does not have | undefined it should, if it does then validatePublisher should support validating an undefined publisher?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are totally right, it's weird because we have different requirements for packaging and publishing but the types are only ensured for publishing currently. I'll create different types and validation for both requirement cases, possibly more debt work to come...

@benibenj benibenj dismissed Tyriar’s stale review August 23, 2024 08:51

proper validation and typing

@benibenj benibenj merged commit 0ec0e7f into main Aug 23, 2024
8 checks passed
@benibenj benibenj deleted the benibenj/calm-angelfish branch August 23, 2024 09:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Publisher required when only packaging an extension
4 participants