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

Remove unnecessary getter #3947

Merged
merged 1 commit into from
Nov 4, 2022
Merged

Conversation

j3r3miah
Copy link
Contributor

@j3r3miah j3r3miah commented Nov 2, 2022

Quick follow up to #3927: @KvanTTT pointed out that the getter was unnecessary, and I confirmed this works fine. cc @parrt

@parrt
Copy link
Member

parrt commented Nov 2, 2022

Hi. hmm...scary. How do you know it's not used by a template somewhere (dyn lookup)?

@j3r3miah
Copy link
Contributor Author

j3r3miah commented Nov 2, 2022

@parrt getSignature() was just added 2 days ago, and a comment after merge pointed out that it's superfluous: https://github.com/antlr/antlr4/pull/3927/files#diff-2d87ee1a79fa64cd1ef7ff3e2557b44dd4ef4aace1d772d89ae2eaa611ef7596R28

Signed-off-by: Jeremiah Boyle <jeremiah.boyle@gmail.com>
@j3r3miah j3r3miah force-pushed the no_go_type_assertions-followup branch from d00521e to 023a488 Compare November 4, 2022 17:26
@j3r3miah
Copy link
Contributor Author

j3r3miah commented Nov 4, 2022

Whoops, fixed missing DCO on commit.

@parrt This felt like a trivial amendment to my previous change but I'm ambivalent about its necessity. Feel free to merge or close as you or @KvanTTT see fit. Thanks!

@parrt
Copy link
Member

parrt commented Nov 4, 2022

Sorry for the delay. Just overwhelmed with multiple open source projects (and work)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants