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

Jump to bottom

Create lro-location-header rule to disable LroLocationHeader LintDiff rule #514

Merged
merged 7 commits into from
Mar 28, 2024
Merged

Create lro-location-header rule to disable LroLocationHeader LintDiff rule #514

merged 7 commits into from
Mar 28, 2024

Conversation

tjprescott
Copy link
Member

@tjprescott tjprescott commented Mar 25, 2024 •
edited
Loading

@azure-sdk
Copy link
Collaborator

azure-sdk commented Mar 25, 2024 •
edited
Loading

All changed packages have been documented.

@azure-sdk
Copy link
Collaborator

You can try these changes at https://cadlplayground.z22.web.core.windows.net/typespec-azure/prs/514/

Check the website changes at https://tspwebsitepr.z22.web.core.windows.net/typespec-azure/prs/514/

@tjprescott tjprescott mentioned this pull request Mar 28, 2024
Closed
@tjprescott tjprescott force-pushed the arm/LroLocationHeaderRule branch 2 times, most recently from c1fad5c to d94e26f Compare March 28, 2024 19:28
timotheeguerin
timotheeguerin reviewed Mar 28, 2024
View reviewed changes
.chronus/changes/arm-LroLocationHeaderRule-2024-2-25-20-46-31.md Outdated Show resolved Hide resolved
docs/libraries/azure-resource-manager/rules/lro-location-header.md Outdated
```tsp
@armResourceOperations
interface Employees {
delete is ArmResourceDeleteAsync<Employee>;
Copy link
Member

Choose a reason for hiding this comment

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

Not sure this example represent what someone could see. As this operation template is already deprecated.
Maybe should should show what a bad custom template would have and in the correct section show both a correct custom template but keep the standard templates as well

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 updated it to use what the actual problem-generated specs used, which was overriding the LroHeaders template parameter with {} to get rid of the headers. Generally, using our templates properly should avoid this scenario.

packages/typespec-azure-resource-manager/src/rules/lro-location-header.ts Outdated Show resolved Hide resolved
@tjprescott tjprescott enabled auto-merge (squash) March 28, 2024 21:59
@tjprescott tjprescott merged commit f1d30de into main Mar 28, 2024
16 checks passed
@tjprescott tjprescott deleted the arm/LroLocationHeaderRule branch March 28, 2024 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@timotheeguerin timotheeguerin timotheeguerin approved these changes

@markcowl markcowl markcowl approved these changes

@bterlson bterlson Awaiting requested review from bterlson bterlson is a code owner

@allenjzhang allenjzhang Awaiting requested review from allenjzhang allenjzhang is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@tjprescott @azure-sdk @timotheeguerin @markcowl