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: add override for /app/installations/ response #388

Merged
merged 4 commits into from
Sep 22, 2023

Conversation

Jman420
Copy link
Contributor

@Jman420 Jman420 commented Sep 5, 2023

Add override for /app/installations/ response for account property to be an allOf list instead of anyOf

Resolves octokit/openapi-types.ts#305


Before the change?

After the change?

Pull request checklist

  • [ n/a ] Tests for the changes have been added (for bug fixes / features)
  • [ n/a ] Docs have been reviewed and added / updated if needed (for bug fixes / features)

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

  • Yes
  • No
  • Maybe? - The override technically changes the type of the response's account property but the existing type is broken as it does not include all of the necessary fields.

@kfcampbell kfcampbell added the Type: Bug Something isn't working as documented label Sep 8, 2023
@Jman420
Copy link
Contributor Author

Jman420 commented Sep 8, 2023

I think the Dry-run is failing due to the build script not handling parsing the version from the filename correctly. I think the regex currently only supports a single digit for the two parts of the version and the download script has downloaded a file for v3.10. Version 3.10 is also not included in the SUPPORTED_GHES_OPERATIONS constant.

Should I include fixes to those issue in this PR?

@kfcampbell
Copy link
Member

@Jman420 unfortunately that's affecting all the PRs at the moment. If you do fix that, would you mind using a separate PR?

@Jman420 Jman420 force-pushed the override/get-app-installations branch from 4b76972 to a4ef13a Compare September 9, 2023 21:52
wolfy1339
wolfy1339 previously approved these changes Sep 15, 2023
scripts/overrides/index.mjs Outdated Show resolved Hide resolved
scripts/overrides/index.mjs Outdated Show resolved Hide resolved
@wolfy1339 wolfy1339 changed the title Add override for /app/installations/ response fix: add override for /app/installations/ response Sep 22, 2023
@wolfy1339 wolfy1339 merged commit 21303bb into octokit:main Sep 22, 2023
6 checks passed
@octokitbot
Copy link
Collaborator

🎉 This PR is included in version 12.1.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@Jman420 Jman420 deleted the override/get-app-installations branch September 22, 2023 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released Type: Bug Something isn't working as documented
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[BUG]: GitHub App Installation DataStructure's account property is missing data fields
4 participants