Maintainability Roadmap

mikeharder commented 4 months ago

A potential roadmap to improve maintainability, ending with the archiving of this repo.

TypeScript

[ ] Update SwaggerBreakingChanges in openapi-alps to call npm exec --no oad instead of @azure/oad@~0.10.6
- https://github.com/Azure/openapi-diff/issues/309
[ ] Move code to azure-rest-api-specs repo, folder eng/tools/openapi-diff
[ ] Align deps and config with typespec-validation, add to eng/package.json
[ ] Upgrade all dependencies to latest
- If some are really difficult we can choose to stay back, but anything older than latest is a liability
[ ] Remove .NET code from package, replace with call to nuget/dotnet to install and run separate .NET package
[ ] Existing CLI name oad seems good and short, and aligns with tsv
- But could rename cli to openapi-diff to be more descriptive
[ ] Deprecate npm package @azure/oad

.NET

[ ] Move code to azure-sdk-tools repo, folder tools/openapi-diff
[ ] Rename to Azure.Sdk.Tools.OpenApiDiff
[ ] Align deps and config from a similar existing tool, perhaps http-fault-injector
[ ] Publish nuget package and/or dotnet tool to devops nuget feed (similar to http-fault-injector)
[ ] Could potentially move code to azure-rest-api-specs, and compile dynamically
- But it would be the first .NET code in the repo
- It would add a dependency on .NET SDK (rather than just .NET runtime)

Docs

[ ] Move docs to azure-rest-api-specs repo

Final

[ ] Archive this repo (Azure/openapi-diff)

konrad-jamrozik commented 4 months ago

Disclaimer: I do not have strong opinions on that and I am OK with the proposed approach.

Re:

Move code to azure-rest-api-specs repo, folder eng/tools/openapi-diff

I guess we decided on the mono-repo model? I do like the standalone repo model for better inner loop development, decoupling and versioning experience.

Copy deps and config from typespec-validation, add to eng/package.json

Why? Shouldn't openapi-diff have only the deps it needs? What if typespec-validation has a dep that openapi-diff doesn't need?

Existing CLI name oad seems good and short, and aligns with tsv

I think we could rethink the name as it leads to confusion with oav and aov (aka azure-openapi-validator) and LintDiff (due to diff)

Update SwaggerLintDiff in openapi-alps to call npm exec --no oad instead of @azure/oad@~0.10.6

Case in my point above: LintDiff is a separate tool unrelated to oad. Related code in openapi-alps calling oad is breaking-change.ts and reakingChangeRuleManager.ts

Move code to azure-sdk-tools repo, folder tools/openapi-diff

The code in openapi-diff conceptually belongs together. It doesn't really sit well with me to split it into two parts and shelve them inside two bigger mono-repos. If we would have use cases where we reuse these two parts independently of each other then it would make more sense to me.

mikeharder commented 4 months ago

Copy deps and config from typespec-validation, add to eng/package.json

Why? Shouldn't openapi-diff have only the deps it needs? What if typespec-validation has a dep that openapi-diff doesn't need?

What I meant here was to align the deps and config, not actually copy. I mean things like align the versions of common dependencies like typescript where we try to stay consistent within a given monorepo.

Existing CLI name oad seems good and short, and aligns with tsv

I think we could rethink the name as it leads to confusion with oav and aov (aka azure-openapi-validator) and LintDiff (due to diff)

IMO the main question here is what do you want devs to use on the command-line? We chose tsv for "typespec-validation" since we thought a short name similar to tsp would be easiest for devs to use. If the command is mostly used in scripts, or we just want to eliminate confusion, then a long name like openapi-diff is fine with me.

Update SwaggerLintDiff in openapi-alps to call npm exec --no oad instead of @azure/oad@~0.10.6

Case in my point above: LintDiff is a separate tool unrelated to oad. Related code in openapi-alps calling oad is breaking-change.ts and reakingChangeRuleManager.ts

Sorry I meant "SwaggerBreakingChange" here.

Move code to azure-rest-api-specs repo, folder eng/tools/openapi-diff Move code to azure-sdk-tools repo, folder tools/openapi-diff

The code in openapi-diff conceptually belongs together. It doesn't really sit well with me to split it into two parts and shelve them inside two bigger mono-repos. If we would have use cases where we reuse these two parts independently of each other then it would make more sense to me.

In my experience, optimizing for ease of maintenance:

If a tool is only used in one repo, put the tool in the repo itself, and build/install the tool as part of "installing the repo toolset". This is how typespec-validation and typespec-requirement work in azure-rest-api-specs, and they are a breeze to maintain.
If a tool is used in multiple repos, or it's written in a "foreign" language and you don't want to add that language's toolset to your repo, put it in a monorepo and share via packages.

Azure / openapi-diff