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

Check spelling using GitHub Actions + Baseline #31810

Closed
wants to merge 55 commits into from

Conversation

danieljurek
Copy link
Member

@danieljurek danieljurek commented Dec 9, 2024

Opening as a draft PR to prevent bringing in a wide audience of reviewers. It might be necessary to use admin capabilities to merge this one.

This PR:

  • Sets baseline spelling configs
    • Use root cspell.json for words that appear correctly spelled and are seen in 10 or more services
    • Use specification/<service>/cspell.json for other errors, flattened to list words instead of targeting specific files with overrides
  • Update documentation
  • Uses GitHub Actions to run spell check:
    • In PRs on changed files
    • On all specification/ files when pushing to targeted branches (matching TSV - All config), a schedule, or when run manually

Example PR with spelling error (PR):

image

Workflow run for same PR with no spelling error (workflow run):

image

Check Spelling - All runs:

Future improvements to Invoke-Cspell.ps1 should include the ability to silence the entire list of files checked.

Copy link

openapi-pipeline-app bot commented Dec 9, 2024

Next Steps to Merge

Next steps that must be taken to merge this PR:
  • ❌ The required check named Swagger SemanticValidation has failed. Refer to the check in the PR's 'Checks' tab for details on how to fix it and consult the aka.ms/ci-fix guide

Copy link

openapi-pipeline-app bot commented Dec 9, 2024

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

Copy link

PR validation pipeline can not start as the pull request is not merged or mergeable - most likely it has merge conflicts.

@danieljurek
Copy link
Member Author

There is currently a merge conflict because of contention in the custom-words.txt file. I'll establish a new baseline before merging this PR.

@danieljurek danieljurek force-pushed the djurek/cspell-baseline branch from 1b44e53 to ce6c8cd Compare December 10, 2024 04:33
Copy link

PR validation pipeline can not start as the pull request is not merged or mergeable - most likely it has merge conflicts.

1 similar comment
Copy link

PR validation pipeline can not start as the pull request is not merged or mergeable - most likely it has merge conflicts.

Copy link

PR validation pipeline can not start as the pull request is not merged or mergeable - most likely it has merge conflicts.

1 similar comment
Copy link

PR validation pipeline can not start as the pull request is not merged or mergeable - most likely it has merge conflicts.

@danieljurek danieljurek force-pushed the djurek/cspell-baseline branch from 114aaca to 4592bf1 Compare December 11, 2024 23:30
@danieljurek danieljurek force-pushed the djurek/cspell-baseline branch from 6826cea to b18e55d Compare December 12, 2024 05:05
Copy link
Member

@mikeharder mikeharder left a comment

Choose a reason for hiding this comment

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

Let's get consensus on the new check name before merging, since I don't want to need to change the GitHub setting more than once.

@azure-sdk
Copy link
Collaborator

azure-sdk commented Dec 17, 2024

API change check

APIView has identified API level changes in this PR and created following API reviews.

Microsoft.AwsConnector
AnomalyDetector
ApiCenter.DataApi
Microsoft.ApiCenter
Microsoft.AppComplianceAutomation
AzureAppConfiguration
Azure.Monitor.LiveMetrics
Astronomer.Astro
Azure.AI.ChatProtocol
DocumentTranslation
TextTranslation
PurviewDataMap
Azure.Batch
Microsoft.EventGrid
Microsoft.EventGrid.SystemEvents
Azure.ProgrammableConnectivity
Microsoft.MachineLearningServices
Microsoft.AzureFleet
Microsoft.AzureLargeInstance
Microsoft.AzureStackHCI
Microsoft.CodeSigning
AzureCommunicationRoutingService
Azure.Communication.MessagesService
Microsoft.Community
Microsoft.ComputeSchedule
Microsoft.ConnectedCache
Microsoft.ContainerStorage
ContentSafety
Microsoft.Contoso
Azure.Contoso.WidgetManager
Microsoft.PortalServices
Microsoft.Portal
Microsoft.DatabaseWatcher
DevCenterService
Microsoft.DeviceRegistry
Microsoft.DocumentDB
DocumentIntelligence
Microsoft.DurableTask
Easm
Microsoft.EdgeZones
Face
Microsoft.ContainerService
GitHub.Network
HealthDataAIServices.DeidServices
Microsoft.HealthDataAIServices
AzureHealthInsights
ImageAnalysis
Microsoft.Impact
Informatica.DataManagement
Microsoft.IoTOperations
Microsoft.IoTOperationsDataProcessor
Microsoft.IoTOperationsMQ
Microsoft.IoTOperationsOrchestrator
Microsoft.KubernetesRuntime
Language.Conversations.Authoring
Language.AnalyzeDocuments
Language.Text
Language.Text.Authoring
Language.Conversations
Microsoft.LoadTestService
Microsoft.App.DynamicSessions
Microsoft.AVS
Microsoft.AzureTerraform
Microsoft.CodeTransparency
Microsoft.DevOpsInfrastructure
Microsoft.Edge
Microsoft.Fabric
Microsoft.ManagedCcf
Microsoft.Monitor
Microsoft.VerifiedId
Neon.Postgres
Microsoft.NetworkAnalytics
Azure.AI.OpenAI.Assistants
Azure.OpenAI
Oracle.Database
Pinecone.VectorDb
Microsoft.PlaywrightTesting.AuthManager
Microsoft.AzurePlaywrightService
Microsoft.ProgrammableConnectivity
Microsoft.HybridConnectivity
Microsoft.Purview
Quantum.Workspace.Services
Qumulo.Storage
SchemaRegistry
Microsoft.ScVmm
Microsoft.SecretSyncController
Microsoft.ServiceNetworking
Microsoft.Speech.VideoTranslation
Microsoft.AzureSphere
SplitIO.Experimentation
Microsoft.StandbyPool
Azure.Developer.TrustedSigning
Microsoft.VoiceServices
Microsoft.Workloads

@danieljurek
Copy link
Member Author

danieljurek commented Jan 8, 2025

Work items from today's discussion:

  • Keep the baseline as it's already done
  • Convert to YAML
  • Commentary in the YAML describing what to do
  • all words lowercase

@danieljurek
Copy link
Member Author

Handled in #32171

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants