-
Notifications
You must be signed in to change notification settings - Fork 74
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
refactor(pass-style): Avoid name convention conflict #2666
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
erights
force-pushed
the
markm-avoid-name-convention-conflict
branch
from
December 20, 2024 00:01
6c64ec1
to
8998ca7
Compare
turadg
reviewed
Dec 20, 2024
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.
turadg
reviewed
Dec 20, 2024
turadg
approved these changes
Dec 20, 2024
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.
👌
mergify bot
pushed a commit
to Agoric/agoric-sdk
that referenced
this pull request
Dec 20, 2024
) closes: #XXXX refs: endojs/endo#2666 ## Description We generally use names of the form `^([A-Z]\w*)I$`, i.e., identifiers beginning with a capital letter and ending with a lone capital "I", for interface guards. However there were some uses of the same form of name for typescript interfaces. Because interfaces and interface guards are adjacent concepts, this could be confusing. This PR fixes this for agoric-sdk. The companion endojs/endo#2666 fixes it for endo. Because the endo names in question were not exported, there is clearly no dependency between these two PRs. ### Security Considerations Consistent naming conventions makes code more reviewable, which is good for security. ### Scaling Considerations none ### Documentation Considerations Consistent naming conventions helps documentation and its readers. ### Testing Considerations none ### Compatibility Considerations Unlike endojs/endo#2666, the type names in question here are exported by their respective modules (and likely packages), so in theory it is possible to break code outside agoric-sdk that depends on these type names. Because these are only type names that are erased prior to execution, this mismatch could only cause static problems, not runtime problems. Further, the names in question ***seem*** only intended as helper types for defining other types. Likely, and dependence outside agoric-sdk will only be to those outer types, not to these helper types, in which case an actual compat problem is unlikely. But possible. ### Upgrade Considerations Because these are only type names that are erased prior to execution, none.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
closes: #XXXX
Agoric/agoric-sdk#10751
Description
We generally use names of the form
^([A-Z]\w*)I$
, i.e., identifiers beginning with a capital letter and ending with a lone capital "I", for interface guards. However there were some uses of the same form of name for typescript interfaces. Because interfaces and interface guards are adjacent concepts, this could be confusing. This PR fixes this for endo. The companion Agoric/agoric-sdk#10751 fixes it for agoric-sdk. Because the endo names in question were not exported, there is clearly no dependency between these two PRs.Security Considerations
Consistent naming conventions makes code more reviewable, which is good for security.
Scaling Considerations
none
Documentation Considerations
Consistent naming conventions helps documentation and its readers.
Testing Considerations
none
Compatibility Considerations
Because these names are not exported, and are only type names erased prior to execution, none.
Upgrade Considerations
Because these are only type names that are erased prior to execution, none.