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.

Sign up for GitHub

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

Jump to bottom

Adding script to modify access control of generated openapi types #555

Merged
merged 2 commits into from
Nov 11, 2024
Merged

Adding script to modify access control of generated openapi types #555

merged 2 commits into from
Nov 11, 2024

Conversation

etoledom
Copy link
Contributor

Closes #537

Description

This is a proposal to select the access control of generated openapi types independent of each other.

This is a swift script which will run before SwiftLint. It's a kind of brute-force approach, matching strings on the file to be replaced, but it's simple and works.

There's a big room for improvements, like:

  • Passing the types to modify via arguments defined in a separate file.
  • Making the script an executable target inside our package.
  • Improvements to the algorithm itself.

Testing Steps

  • Run make generate
  • No changes to the generated files should be present.

@etoledom etoledom self-assigned this Oct 30, 2024
@wpmobilebot
Copy link

Gravatar UIKit Prototype Build📲 You can test the changes from this Pull Request in Gravatar UIKit Prototype Build by scanning the QR code below to install the corresponding build.
App NameGravatar UIKit Prototype Build Gravatar UIKit Prototype Build
Build Number1653
Version1.0
Bundle IDcom.automattic.gravatar-sdk-demo-uikit.prototype-build
Commitbbfe01f
App Center BuildGravatar SDK Demo - UIKit #342
 Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@wpmobilebot
Copy link

Gravatar SwiftUI Prototype Build📲 You can test the changes from this Pull Request in Gravatar SwiftUI Prototype Build by scanning the QR code below to install the corresponding build.
App NameGravatar SwiftUI Prototype Build Gravatar SwiftUI Prototype Build
Build Number1653
Version1.0
Bundle IDcom.automattic.gravatar-sdk-demo-swiftui.prototype-build
Commitbbfe01f
App Center BuildGravatar SDK Demo - SwiftUI #337
 Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

pinarol
pinarol approved these changes Oct 31, 2024
View reviewed changes
Copy link
Contributor

@pinarol pinarol left a comment

Choose a reason for hiding this comment

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

This should give the desired outcome so looks good to me 👍.

I was thinking about pre-processing the yaml to mark the models as public/internal and making the mustache files respect that. But I don't have any intentions to over engineer this. So 👍

@etoledom etoledom marked this pull request as ready for review November 11, 2024 10:51
@etoledom
Copy link
Contributor Author

etoledom commented Nov 11, 2024 •
edited
Loading

This should give the desired outcome so looks good to me 👍.

I was thinking about pre-processing the yaml to mark the models as public/internal and making the mustache files respect that. But I don't have any intentions to over engineer this. So 👍

Thank you @pinarol !

I've made the PR ready to review. @andrewdmontgomery - Anything to add?

I think it would be good to merge this before we need to update the spec again. If it's good enough now, we can think of further improvements later on if/when they are needed.

@wpmobilebot
Copy link

Gravatar Prototype Build📲 You can test the changes from this Pull Request in Gravatar Prototype Build by scanning the QR code below to install the corresponding build.
App NameGravatar Prototype Build Gravatar Prototype Build
Build Number1678
Version1.0
Bundle IDcom.automattic.gravatar-sdk-demo-uikit.prototype-build
Commitc60f279
App Center BuildGravatar SDK Demo - UIKit #359
 Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@andrewdmontgomery
Copy link
Contributor

I like that this is written in Swift.

Yeah, this is a good start. Let's merge it.

@etoledom etoledom merged commit b503e02 into trunk Nov 11, 2024
12 checks passed
@etoledom etoledom deleted the etoledom/select-internal-openapi-types branch November 11, 2024 22:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pinarol pinarol pinarol approved these changes

Assignees

@etoledom etoledom

Labels
tooling
Projects
None yet
Milestone
Milestone 5 - Quick Editor - M2
Development

Successfully merging this pull request may close these issues.

Generate some OpenAPI models as internal
4 participants
@etoledom @wpmobilebot @andrewdmontgomery @pinarol