-
Notifications
You must be signed in to change notification settings - Fork 175
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
Add continueOnError to create emitter-package-lock #8672
base: main
Are you sure you want to change the base?
Conversation
@@ -242,6 +242,7 @@ stages: | |||
arguments: > | |||
-EmitterPackageJsonPath "$(buildArtifactsPath)/emitter-package.json" | |||
-OutputDirectory "$(Build.ArtifactStagingDirectory)" | |||
continueOnError: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Won't this cause us to potentially ignore real issues as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it might be, but I put the justification in the description that "we are now depending on @typespec/http-client-csharp which is in the TypeSpec repo" so that we cannot bump the version in @typespec/http-client-csharp. We currently don't have a way to fix the error here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Continue on error at least let the pipeline run. And the pipeline runs with reading dev version because we override it at the root, therefore the run result is reliable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hallipr can you help them solve this in another way? I would really like to avoid us blindly ignoring errors that is just going to lead to other issues.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also dislike ignoring the errors from npm list
. We need to resolve the dependency issues to ensure we're testing the appropriate dependencies in the TypeSpec next runs.
We are seeing this error for a long time. https://dev.azure.com/azure-sdk/internal/_build/results?buildId=3976469&view=logs&j=739bb7ec-7296-5b40-7dff-ee3297bfdaa4&t=164b9cb9-850d-5194-a052-63f5151be232
Now it cannot be handled at autorest.csharp side because we are now depending on @typespec/http-client-csharp which is in the TypeSpec repo. As a result we could see the error message
As suggested by @mikeharder , I think adding "continue on error" is the most appropriate/acceptable way to us now, since this error not impact code generation.