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

Updating release-winget according to manifest schema changes #332

Merged
merged 14 commits into from
Apr 29, 2021
Merged

Updating release-winget according to manifest schema changes #332

merged 14 commits into from
Apr 29, 2021

Conversation

ldennington
Copy link
Contributor

@ldennington ldennington commented Apr 28, 2021

It appears that changes have been made to the winget schema since this workflow was created, so I've made the necessary updates here. Additionally, we've fixed a couple issues in the mjcheetham/update-winget task that had been causing this workflow to fail (see this and this).

With these updates, we should be good to go to publish gcm core to winget automatically with the next release. Exciting times!

I validated this workflow locally by successfully opening a PR against my winget-playground repo and running winget validate to ensure the generated manifest adheres to winget's required schema.

Copy link
Contributor

@derrickstolee derrickstolee left a comment

Choose a reason for hiding this comment

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

Wow, they really broke backwards compatibility with these manifest changes. Do you have a link to the full spec? I wonder if there are some other fields we might want to populate (do Description or LongDescription exist?).

.github/workflows/release-winget.yaml Outdated Show resolved Hide resolved
License: Copyright (C) Microsoft Corporation
Description: Secure, cross-platform Git credential storage with authentication to GitHub, Azure Repos, and other popular Git hosting services.
ShortDescription: Secure, cross-platform Git credential storage with authentication to GitHub, Azure Repos, and other popular Git hosting services.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there now a longer Description: that we should populate somehow?

Copy link
Contributor Author

@ldennington ldennington Apr 29, 2021

Choose a reason for hiding this comment

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

Description does still exist. However, for reasons that aren't super clear, ShortDescription is now required, while Description is not. I'm happy to take a stab at adding a longer Description though!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there anything specific we'd want to call out in a longer Description?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can wait on Description until we know more about where it will show up.

@ldennington
Copy link
Contributor Author

ldennington commented Apr 29, 2021

Wow, they really broke backwards compatibility with these manifest changes. Do you have a link to the full spec? I wonder if there are some other fields we might want to populate (do Description or LongDescription exist?).

Yep. I was pretty shocked tbh. Here's the link to the spec, and some documentation around it that I felt was helpful. I'm happy to add whichever extra fields we feel are helpful!

@derrickstolee
Copy link
Contributor

Wow, they really broke backwards compatibility with these manifest changes. Do you have a link to the full spec? I wonder if there are some other fields we might want to populate (do Description or LongDescription exist?).

Yep. I was pretty shocked tbh. Here's the link to the spec, and some documentation around it that I felt was helpful. I'm happy to add whichever extra fields we feel are helpful!

Ah, there seems to be a Description item in addition to the ShortDescription.

with:
token: ${{ secrets.WINGET_TOKEN }}
repo: microsoft/winget-pkgs
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we still want repo: microsoft/winget-pkgs, right? Or, is that built-in to v1.2?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's the default! However, we can keep if it's better to be explicit.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks! Using the default works for me.

This comment was marked as spam.

@ldennington ldennington merged commit 227226e into git-ecosystem:master Apr 29, 2021
@git-ecosystem git-ecosystem deleted a comment from APACGAMONDE Apr 30, 2021
@mjcheetham mjcheetham mentioned this pull request May 6, 2021
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.

None yet

3 participants