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

torchfix: Refactor ERROR_CODE to be consistent #46

Merged
merged 4 commits into from
Apr 22, 2024

Conversation

seemethere
Copy link
Contributor

@seemethere seemethere commented Apr 22, 2024

Refactors ERROR_CODE --> ERROR, introduces a new TorchError class that defines what an error is including its code + the message it's supposed to have since I found that most errors were defined by the Code at the top + a string message that was semi-hard to find amongst the code.

This also makes it so that ERRORS needs to be a list instead of being optionally a list or a value to make it more consistent.

Relates to #6 since I'll use this backing to auto-generate documentation for all of the error codes we support

Refactors ERROR_CODE --> ERROR, introduces a new TorchError class that
defines what an error is including its code + the message it's supposed
to have since I found that most errors were defined by the Code at the
top + a string message that was semi-hard to find amongst the code.

This also makes it so that ERRORS needs to be a list instead of  being
optionally a list or a value to make it more consistent.

Signed-off-by: Eli Uriegas <[email protected]>
@seemethere seemethere requested a review from kit1980 April 22, 2024 18:51
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 22, 2024
Signed-off-by: Eli Uriegas <[email protected]>
Signed-off-by: Eli Uriegas <[email protected]>
Signed-off-by: Eli Uriegas <[email protected]>
@kit1980
Copy link
Contributor

kit1980 commented Apr 22, 2024

Thanks.
I'm a bit unsure about "makes it so that ERRORS needs to be a list instead of being optionally a list or a value", because many rules have just one error code, so now it's a lot of [0] added. But it's probably fine.

@seemethere
Copy link
Contributor Author

Thanks. I'm a bit unsure about "makes it so that ERRORS needs to be a list instead of being optionally a list or a value", because many rules have just one error code, so now it's a lot of [0] added. But it's probably fine.

I think consistency here is the more important part because having an either or scenario can make it a bit confusing.

@seemethere seemethere merged commit b2d55f8 into main Apr 22, 2024
2 checks passed
@seemethere seemethere deleted the seemethere/consistent_errors branch April 22, 2024 19:16
kit1980 added a commit that referenced this pull request Apr 24, 2024
kit1980 added a commit that referenced this pull request Apr 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants