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: support constructor with default arguments #74

Conversation

damianogiusti
Copy link
Contributor

Description

This PR implements the requested feature of skipping constructor parameters that are declared with a default value (#54).

Changelist:

  • Add name and hasDefault values to the KoinMetaData.ConstructorParameter class to keep track of those info during code generation;
  • Filter out constructor parameters that have a default value;
  • Add the param name when generating the definition to disambiguate when parameters with default values are places between others that don't have default values.

@damianogiusti
Copy link
Contributor Author

Hi @arnaudgiuliani, do you think this feature will be a good candidate to be included in the next Koin Annotations release? I believe it will be useful for a lot of users out there. Thanks!

@arnaudgiuliani
Copy link
Member

thanks for you proposal, interesting case. I will take time to review it 👍

@arnaudgiuliani arnaudgiuliani self-requested a review March 23, 2023 17:37
@arnaudgiuliani arnaudgiuliani added this to the 1.2.1 milestone Mar 23, 2023
@arnaudgiuliani arnaudgiuliani modified the milestones: 1.2.1, 1.3.0 May 10, 2023
@arnaudgiuliani
Copy link
Member

Reschedule it for next version, not 1.2.1 as it's for fixes of 1.2 👍

@arnaudgiuliani arnaudgiuliani changed the base branch from main to 1.3.0 September 4, 2023 15:54
@arnaudgiuliani arnaudgiuliani deleted the branch InsertKoinIO:1.3.0 September 6, 2023 09:42
@arnaudgiuliani arnaudgiuliani reopened this Sep 6, 2023
@arnaudgiuliani
Copy link
Member

your work is transferred on feature/constructor-with-default-arguments, I have solved the conflicts and breakings.

@arnaudgiuliani
Copy link
Member

PR is continued in #91

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal status:checking Ticket is currently being checked type:improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants