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

[docs] Add to "Swagger LintDiff for TypeSpec" #27451

Merged
merged 3 commits into from
Jan 26, 2024
Merged

[docs] Add to "Swagger LintDiff for TypeSpec" #27451

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
de892e3
[docs] Add to "Swagger LintDiff for TypeSpec"
mikeharder Jan 18, 2024
1194d05
Merge branch 'main' into ci-fix-lintdiff-tsp
mikeharder Jan 23, 2024
007c0a6
Fix bad merge
mikeharder Jan 23, 2024
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
4 changes: 4 additions & 0 deletions documentation/ci-fix.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,10 @@ We believe this is a false positive: https://github.com/Azure/azure-openapi-vali

We believe this is a false positive: https://github.com/Azure/azure-openapi-validator/issues/642. Until fixed, spec authors should **not** suppress the violations in `readme.md`, but rather have label `Approved-LintDiff` applied to their PR to ignore the errors.

### `@singleton` causes `EvenSegmentedPathForPutOperation` and `XmsPageableForListCalls`

If `EvenSegmentedPathForPutOperation` and/or `XmsPageableForListCalls` are failing for OpenAPI generated from TypeSpec using `@singleton` (OpenAPI path ends with `/default`), we believe this is a false positive: https://github.com/Azure/azure-openapi-validator/issues/646. Until fixed, spec authors should **not** suppress the violations in `readme.md`, but rather have label `Approved-LintDiff` applied to their PR to ignore the errors.
Copy link

@konrad-jamrozik konrad-jamrozik Jan 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Suggestion:

Replace:

but rather have label Approved-LintDiff applied to their PR to ignore the errors.

With:

but rather suppress add appropriate label per https://aka.ms/azsdk/pr-suppressions.

Copy link

@konrad-jamrozik konrad-jamrozik Jan 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

(to avoid duplication and teach PR authors about the docs they should know)

Copy link
Member Author

Choose a reason for hiding this comment

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

I understand the goal to avoid duplicating the Approved-LintDiff label. However, if we add this link from ci-fix to pr-suppressions, I think we are creating a loop. Because in the Suppression cases table in page pr-suppressions, the value in column PR suppression for row Swagger LintDiff is a link back to ci-fix.

So a user goes to ci-fix, is told they need a PR-level suppression which is documented at pr-suppressions, but then the row for Swagger LintDiff directs them back to ci-fix?

I'm inclined to keep my original text rather than create a loop in our docs.

Copy link

Choose a reason for hiding this comment

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

Ok, cool. I plan to move this ci-fix file to azsdkdocs anyway, so I can fix this loop while I do that.


## `Swagger Avocado`

>[!IMPORTANT]
Expand Down