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

Support Unimplemented attribute #826

Merged
merged 7 commits into from
Nov 9, 2023
Merged

Support Unimplemented attribute #826

merged 7 commits into from
Nov 9, 2023

Conversation

swernli
Copy link
Collaborator

@swernli swernli commented Oct 30, 2023

This change introduces the @Unimplemented() attributed which can be used to mark an item as something that should not be used. The motivation behind including this and not just treating it as a Not Found error is to give the user more information that this API may have previously existing and is not currently supported (see #699 for context).

@swernli swernli changed the title Swernli/unimplemented-attr Support Unimplemented attribute Oct 30, 2023
@swernli
Copy link
Collaborator Author

swernli commented Oct 30, 2023

whoops... looks like I somehow merged @DmitryVasilevsky 's changes with my own here. I'll try to fix this up...

@swernli swernli changed the base branch from swernli/impl-attrs to main October 30, 2023 22:54
@swernli swernli force-pushed the swernli/unimplemented-attr branch from d3d32fe to 97a5024 Compare October 30, 2023 22:56
Copy link
Member

@minestarks minestarks left a comment

Choose a reason for hiding this comment

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

This implementation LGTM. Thank you!

Since we had that discussion about the fate of @Config() recently, looking at this PR with fresh eyes, this feature seems more-or-less equivalent to the @Config() attribute. We could define a special Config expression that should never be true, like @Config(Unimplemented). Would reduce complexity (since it would just get handled along with all the other @Configged items in the preprocessing step) and I think the behavior would be ... the same? Minus some very specific error reporting... which could be worth the added complexity.

That's my current take on the feature itself, but I'm happy whichever way you decide to go. Thanks for all the revisions and discussion!

swernli and others added 5 commits November 9, 2023 10:41
This change introduces the @unimplemented() attributed which can be used to mark an item as something that should not be used. The motivation behind including this and not just treating it as a Not Found error is to give the user more information that this API may have previously existing and is not currently supported (see #699 for context).
@swernli swernli force-pushed the swernli/unimplemented-attr branch from c823d7c to d2fa415 Compare November 9, 2023 18:42
@swernli swernli enabled auto-merge November 9, 2023 20:39
@swernli swernli added this pull request to the merge queue Nov 9, 2023
Merged via the queue into main with commit d0f6102 Nov 9, 2023
@swernli swernli deleted the swernli/unimplemented-attr branch November 9, 2023 21:21
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