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

Update spec PR review workflow diagram: simplify + make it also applicable to data-plane. #28930

Merged
merged 3 commits into from
May 2, 2024

Conversation

konrad-jamrozik
Copy link

@konrad-jamrozik konrad-jamrozik commented May 1, 2024

This PR, before it can be merged, requires changes to aka.ms/azsdk/specreview/merge because currently that URL refers to text in the diagram that is being removed by this PR. I also need to review any mentions of the diagram in the openapi-alps code to ensure it is not referencing information erased by this PR.

Prerequisite PRs:

This PR addresses:

This PR builds on recent improvements made to Next Steps to Merge comment:

  • Pull Request 547143: Add "NotReadyForArmReview" label handling; fix "RPaasException" label handling; rework requiredLabelsRules

The new diagram:

spec_pr_review_workflow_diagram

The diagram was updated based on these instructions:

Copy link

openapi-pipeline-app bot commented May 1, 2024

Next Steps to Merge

✅ All automated merging requirements have been met! To get your PR merged, see aka.ms/azsdk/specreview/merge.

Copy link

openapi-pipeline-app bot commented May 1, 2024

Swagger Validation Report

️️✔️BreakingChange succeeded [Detail] [Expand]
There are no breaking changes.
️️✔️Breaking Change(Cross-Version) succeeded [Detail] [Expand]
There are no breaking changes.
️️✔️CredScan succeeded [Detail] [Expand]
There is no credential detected.
️️✔️LintDiff succeeded [Detail] [Expand]
Validation passes for LintDiff.
️️✔️Avocado succeeded [Detail] [Expand]
Validation passes for Avocado.
️️✔️SwaggerAPIView succeeded [Detail] [Expand]
️️✔️TypeSpecAPIView succeeded [Detail] [Expand]
️️✔️ModelValidation succeeded [Detail] [Expand]
Validation passes for ModelValidation.
️️✔️SemanticValidation succeeded [Detail] [Expand]
Validation passes for SemanticValidation.
️️✔️PoliCheck succeeded [Detail] [Expand]
Validation passed for PoliCheck.
️️✔️SpellCheck succeeded [Detail] [Expand]
Validation passes for SpellCheck.
️️✔️Lint(RPaaS) succeeded [Detail] [Expand]
Validation passes for Lint(RPaaS).
️️✔️PR Summary succeeded [Detail] [Expand]
Validation passes for Summary.
️⌛Automated merging requirements met pending [Detail]
Posted by Swagger Pipeline | How to fix these errors?

Copy link

openapi-pipeline-app bot commented May 1, 2024

Swagger Generation Artifacts

️️✔️ApiDocPreview succeeded [Detail] [Expand]
Posted by Swagger Pipeline | How to fix these errors?

Copy link

openapi-pipeline-app bot commented May 1, 2024

PR validation pipeline restarted successfully. If there is ApiView generated, it will be updated in this comment.

@konrad-jamrozik konrad-jamrozik added Central-EngSys This issue is owned by the Engineering System team. Spec PR Tools Tooling that runs in azure-rest-api-specs repo. labels May 1, 2024
@konrad-jamrozik konrad-jamrozik merged commit c7c1ce7 into main May 2, 2024
32 checks passed
@konrad-jamrozik konrad-jamrozik deleted the users/kojamroz/new_diagram branch May 2, 2024 06:47
@maririos
Copy link
Member

maririos commented May 2, 2024

@konrad-jamrozik Looking into consolidating information across , in the diagram Step #2 "ARM or data-plane review` the message doesn't talk about data plane review at all. Is this because the message is no in the Next steps to Merge?
If so, should the ARM process be included in the Next Steps to merge (it probably already is) so should it be removed from this diagram?

@konrad-jamrozik
Copy link
Author

I discussed this with @maririos offline. I didn't mention data-plane review because it was a bit too complex and I wanted to keep the diagram lightweight. But perhaps I can figure out a way to update the diagram to make it clear it is ARM xor data-plane.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Central-EngSys This issue is owned by the Engineering System team. Spec PR Tools Tooling that runs in azure-rest-api-specs repo.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants