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

feat: Generate Models in OctoKit.GraphQL to introduce new Repository.PlannedFeatures model #316

Merged

Conversation

SebastianO1717
Copy link
Contributor

In the following Issue #8414 the work for promoting a repo_plan_features_api was introduced by the following PR #319602


Before the change?

Before this PR, the RepositoryPlanFeatures Model did not exists.

After the change?

After this change, the RepositoryPlanFeatures introduced by @talum will be available in the newer versions of the Octokit.GraphQL nuget package.

Pull request checklist

  • Tests for the changes have been added (for bug fixes / features) - Integration tests have been ran as per instructions in the following doc

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

  • Yes
  • No

Copy link

github-actions bot commented Apr 9, 2024

👋 Hi! Thank you for this contribution! Just to let you know, our GitHub SDK team does a round of issue and PR reviews twice a week, every Monday and Friday! We have a process in place for prioritizing and responding to your input. Because you are a part of this community please feel free to comment, add to, or pick up any issues/PRs that are labled with Status: Up for grabs. You & others like you are the reason all of this works! So thank you & happy coding! 🚀

@SebastianO1717
Copy link
Contributor Author

A few questions for contributors of the repo:

Is it okay that 50 of the integration tests did not run? If not, is there a way to force these to run through specific configurations?

Image displaying the optional 50 tests that didn't run
image

Also, I'm open to suggestions in regard to seeing if its possible to generate a smaller set of models. I see there are a lot of changes to them, but on our end we are more focused on the addition of the RepositoryPlanFeatures model being added.

cc: @talum

@nickfloyd nickfloyd changed the title Generate Models in OctoKit.GraphQL to introduce new Repository.PlannedFeatures model feat: Generate Models in OctoKit.GraphQL to introduce new Repository.PlannedFeatures model Apr 9, 2024
@nickfloyd nickfloyd added the Type: Feature New feature or request label Apr 9, 2024
@nickfloyd
Copy link
Contributor

Is it okay that 50 of the integration tests did not run? If not, is there a way to force these to run through specific configurations?

Hey @SebastianO1717, thanks for this change. Just as a level set, this particular SDK has never made the technical transition out of beta. We were hoping to make that transition after we updated the framework / .NET targets but we deprioritized most of that in favor of focusing on Generative SDKs.

That said, we'd welcome any changes that you'd like to make here but remember there are some rough spots in the implementation (i.e. not all integration tests run - and coverage is a bit low) and generation, as you discovered, is all encompassing and will run for all changes to the GitHub GraphQL schema. The generator also breaks occasionally - there are a number of complexities that exist with generating SDKs from a GraphQL schema.

Again, all things that we would've liked to address, but in favor of a newer more stable approach we decided to set this aside while we see if we can solve the use case that this SDK attempts to fill in a more consistent way.

@nickfloyd nickfloyd merged commit 03faae6 into octokit:main Apr 9, 2024
4 checks passed
@SebastianO1717 SebastianO1717 deleted the dev/sebastiano1717/update_graphql branch April 9, 2024 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants