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(presets): switch kiota monorepo to group #32437

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

melotic
Copy link
Contributor

@melotic melotic commented Nov 10, 2024

Changes

Added other kiota repositories to the kiota monorepo preset.

Context

#32164 added the kiota repository, but it also spans multiple repos for each language it supports.

Documentation (please check one with an [x])

  • I have updated the documentation, or
  • No documentation update is required

How I've tested my work (please select one)

I have verified these changes via:

  • Code inspection only, or
  • Newly added/modified unit tests, or
  • No unit tests but ran on a real repository, or
  • Both unit tests + ran on a real repository

Copy link
Collaborator

@rarkins rarkins left a comment

Choose a reason for hiding this comment

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

Are the versions between repos kept in-sync? If not then this is probably not right for a monorepo grouping and instead might be part of group:recommended, or just a standalone grouping of its own

@melotic
Copy link
Contributor Author

melotic commented Nov 11, 2024

Are the versions between repos kept in-sync? If not then this is probably not right for a monorepo grouping and instead might be part of group:recommended, or just a standalone grouping of its own

They are not, only the version in each repo are in sync. I'll port this over to group:recommended.

@melotic melotic changed the title fix(presets): include other kiota repos fix(presets): switch kiota monorepo to group Nov 11, 2024
@melotic melotic requested a review from rarkins November 11, 2024 17:20
Copy link
Member

@viceice viceice left a comment

Choose a reason for hiding this comment

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

I don't like this new default grouping.

those packages should be grouped by their monorepo when the repo published them with the same version.

the other groups should be fully optional and opt-in, because they create immortal PRs.

@melotic
Copy link
Contributor Author

melotic commented Nov 11, 2024

I don't like this new default grouping.

those packages should be grouped by their monorepo when the repo published them with the same version.

the other groups should be fully optional and opt-in, because they create immortal PRs.

Is what you're saying define multiple monorepos for each Kiota repo and an optional group to group all the monorepos? That makes sense.

For context, each kiota repo publishes multiple packages, versioned together, but the versions are not the same across repos. (i.e., the .NET packages are at 1.14.0, while the NPM ones are at 1.0.0-preview.75)

@rarkins
Copy link
Collaborator

rarkins commented Nov 12, 2024

What % of users would you estimate want to update them all together at once? If it's very high, then adding it to group:recommended makes sense. If it's not high, and this is more of an "opinionated" grouping, then it is better to stand on its own.

@viceice
Copy link
Member

viceice commented Nov 12, 2024

i think most user are only using one tech stack, eg npm or dotnet on a single repo

@melotic
Copy link
Contributor Author

melotic commented Nov 13, 2024

Hard to say exactly, Kiota is fairly new. I would guesstimate that 50% of users have a dependency on the Kiota library itself (microsoft/kiota) to generate OpenAPI schemas & bindings, and then those bindings have a dependency on say microosft/kiota-typescript or microsoft/kiota-dotnet. The other half are just consumers without a need to generate bindings.

Most would only have one language like @viceice mentioned for the bindings. In my case, it's a C# backend with a TypeScript frontend.

There's an argument that any updates to the generation code (microsoft/kiota) should also cause an update to the code generated microsoft/kiota-typscript.

But I as a user would be okay with the default behavior for 2 PRs, one for the actual generation kiota libraries, and another for the bindings.

@astellingwerf
Copy link
Collaborator

Doesn't this change technically demand for a migration step? Folks might reference "monorepo:kiota", but it won't resolve any longer.

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.

4 participants