Implemented no-amplify-errors in packages where it should be active #1973
Conversation
🦋 Changeset detectedLatest commit: f87b76d The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| @@ -20,6 +20,7 @@ export const getBackendOutputWithErrorHandling = async ( | |||
| error instanceof BackendOutputClientError && | |||
| error.code === BackendOutputClientErrorType.DEPLOYMENT_IN_PROGRESS | |||
| ) { | |||
| // eslint-disable-next-line amplify-backend-rules/no-amplify-errors | |||
There was a problem hiding this comment.
Now that I see the code around the suppression I do wonder what we should do about this long term.
- Should we be moving the error handling like this up? I.e. move this to relevant CLI commands instead of having this handling here?
- Should we re-classify this package (and other
X-generators) and applyprefer-amplify-errorsinstead and keep local mapping at this level?
I would say 1. seems to be more correct. And I think lifting all these mappings to cli would be applicable to any command that pulls outputs. These don't seem to be specific to model generation.
@Amplifiyer @rtpascual what do you think?
There was a problem hiding this comment.
I'm leaning towards 1 as well since this is already done for the ampx generate form command https://github.com/aws-amplify/amplify-backend/blob/main/packages/cli/src/commands/generate/forms/generate_forms_command.ts#L75-L137.
I believe moving this file to cli will get rid of the need for the duplicate code for forms as a side effect.
There was a problem hiding this comment.
Btw. if we align on number 1 then this PR should go as is and work item should be opened to move this handling up separately.
Problem
no-amplify-errorsshould be active in more packages than justauth-constructIssue number, if available: N/A
Changes
Implemented
no-amplify-errorslint rule in most packages where it should be active.Only packages that are excluded which will eventually have
no-amplify-errorsactivated in areplatform-coreandclient-config-- has too many errors to be included in this PR.model-generatorandschema-generatorhad some violations, but all their violations seem to be fine to leave as AmplifyUserError (added in line ignores for them).Corresponding docs PR, if applicable:
Validation
linter is happy with the updates
Checklist
run-e2elabel set.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.