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

Feature/add typed method list abbreviation #153

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

unnamedunknownusername
Copy link

@unnamedunknownusername unnamedunknownusername commented May 17, 2023

WIP for #150

Essentially just copying the code from MethodList and creating TypedMethodList

Then inserting a copy of a section of typed signatures into the right spot

@MichaelHatherly, let me know if I have missed anything

local groups = methodgroups(func, typesig, modname; exact = false)
if !isempty(groups)

#START OF COPIED CODE FROM TYPEDMETHODSIGNATURES
Copy link
Member

Choose a reason for hiding this comment

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

@unnamedunknownusername could this code just be refactored out into a reusable function rather than copying it across?

Choose a reason for hiding this comment

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

will do

Choose a reason for hiding this comment

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

@MichaelHatherly Have seperated out repeated code into a resusable function let me know if I have missed anything

@MichaelHatherly
Copy link
Member

(Will need some tests as well before merging is possible.)

@unnamedunknownusername
Copy link
Author

unnamedunknownusername commented May 27, 2023

(Will need some tests as well before merging is possible.)

I have added a section in the tests for typed method list testset. But I am thinking we probably need more/want more comprehensive ones

Also how can I run the tests locally/verify that the ones I add are passing.

Sorry if those are simple questions, just having a little difficulty with this part

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.

2 participants