Skip to content

Conversation

@im4xpro
Copy link

@im4xpro im4xpro commented Feb 22, 2024

this resolves a problem in one of our projects.

}

// Prepare parameter string, auto-nil for optional values
let defaultValue = (typeAnnotationText.contains("?") || typeAnnotationText.contains("= nil")) ? " = nil" : ""
Copy link
Collaborator

Choose a reason for hiding this comment

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

@Lutzifer I woudl also like to have your opionion on that.

I understand that this solves a problem for one of our projects. With the change regarding closures i am pretty fine, but regarding the "auto-nil"-defaultvalue i am not sure if that always makes sense or creates some kind of trap for us as an uninitialized value is not complained anymore as soon as it is optional

Copy link
Member

Choose a reason for hiding this comment

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

I second that.

@im4xpro

  • restore the original test, we should not have to change this to ensure backwards-compatibilty
  • add a new test that documents what changes we need
  • implement the macro so that both cases are successfull

Copy link
Member

@Lutzifer Lutzifer left a comment

Choose a reason for hiding this comment

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

See comments

@Lutzifer
Copy link
Member

closing this for now

@Lutzifer Lutzifer closed this Apr 10, 2024
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