Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion eng/tools/lint-diff/src/processChanges.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { join, relative, resolve, sep } from "path";

Check failure on line 1 in eng/tools/lint-diff/src/processChanges.ts

View workflow job for this annotation

GitHub Actions / Protected Files

File 'eng/tools/lint-diff/src/processChanges.ts' should only be updated by the Azure SDK team. If intentional, the PR may be merged by the Azure SDK team via bypassing the branch protections.
import { readFile } from "fs/promises";
import { pathExists } from "./util.js";
import { specification, readme, swagger } from "@azure-tools/specs-shared/changed-files";
Expand Down Expand Up @@ -135,9 +135,18 @@
// For readme files that have changed but there are no affected swaggers,
// add them to the map with no tags
for (const changedReadme of existingChangedFiles.filter(readme)) {
const readmePath = resolve(rootPath, changedReadme);

// Skip readme.md files that don't have "input-file:" as autorest cannot
Copy link
Member

Choose a reason for hiding this comment

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

Is this true? I thought https://aka.ms/autorest or the yaml blocks were the markers?

Copy link
Member

@mikeharder mikeharder Jun 9, 2025

Choose a reason for hiding this comment

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

Check is here in autorest:

https://github.com/Azure/autorest/blob/e57564401c49ed6eb6085d5c474dceb772f47391/packages/extensions/core/src/lib/autorest-core.ts#L153

I did add https://aka.ms/autorest to the RPaaS readme, to make Avocado pass. Another option here, might be to remove https://aka.ms/autorest from the RPaaS readme, which might make LintDiff think it's a no-op, but then we need to patch Avocado.

Sample readme: https://github.com/Azure/azure-rest-api-specs/blob/547503202d2e64df5b4d73bab6d64914133aee66/specification/widget/resource-manager/readme.md?plain=1

I think the decision point, should this readme.md we need for RPaaS be an "autorest readme" or not? If yes, we make this patch to LintDiff. If no, then we remove the url, and patch Avocado. But patching Avocado might be trickier, because we do want Avocado to fail if the URL is missing from a service readme.

Since autorest fails on the file (due to no input files), maybe it's better to make it a non-autorest file?

Finally, we hope the RPaaS readme is a temporary solution that can be removed in 6 months.

Copy link
Member

Choose a reason for hiding this comment

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

I tried removing the autorest marker, but LintDiff still fails, so we might as well keep the marker to avoid needing to patch avocado as well.

// scan them.
const readmeContent = await readFile(readmePath, { encoding: "utf-8" });
if (!readmeContent.includes("input-file:")) {
continue;
}

const service = specModels.get(getService(changedReadme))!;
const readmes = await service.getReadmes();
const readmeObject = readmes.get(resolve(rootPath, changedReadme))!;
const readmeObject = readmes.get(readmePath)!;

if (!changedFileAndTagsMap.has(changedReadme)) {
changedFileAndTagsMap.set(changedReadme, {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# Widget

Check failure on line 1 in eng/tools/lint-diff/test/fixtures/buildState/specification/no-input-file/readme.md

View workflow job for this annotation

GitHub Actions / Protected Files

File 'eng/tools/lint-diff/test/fixtures/buildState/specification/no-input-file/readme.md' should only be updated by the Azure SDK team. If intentional, the PR may be merged by the Azure SDK team via bypassing the branch protections.

> see https://aka.ms/autorest
> This is the AutoRest configuration file for Widget.

## Configuration

Required if any services under this folder are RPaaS.

```yaml
openapi-type: arm
openapi-subtype: rpaas
```
9 changes: 9 additions & 0 deletions eng/tools/lint-diff/test/processChanges.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { test, describe, expect } from "vitest";

Check failure on line 1 in eng/tools/lint-diff/test/processChanges.test.ts

View workflow job for this annotation

GitHub Actions / Protected Files

File 'eng/tools/lint-diff/test/processChanges.test.ts' should only be updated by the Azure SDK team. If intentional, the PR may be merged by the Azure SDK team via bypassing the branch protections.

import {
getAffectedServices,
Expand Down Expand Up @@ -249,4 +249,13 @@
),
).not.toThrow();
});

test.skipIf(isWindows())("does not include readme files that has no input-file:", async () => {
const actual = await buildState(
["specification/no-input-file/readme.md"],
"test/fixtures/buildState/",
);

expect(actual).toEqual([new Map<string, ReadmeAffectedTags>(), []]);
});
});
Loading