Skip to content
This repository has been archived by the owner on Sep 28, 2024. It is now read-only.

Added convenience function builders for SimpleXXXProperty classes #935

Merged
merged 1 commit into from
Mar 6, 2019

Conversation

LittleLightCz
Copy link
Contributor

No description provided.

@LittleLightCz LittleLightCz changed the title #933 Added convenience function builders for SimpleXXXProperty classes Added convenience function builders for SimpleXXXProperty classes Mar 5, 2019
@edvin
Copy link
Owner

edvin commented Mar 5, 2019

Very nice! Will you add a note to CHANGELOG as well?

@edvin
Copy link
Owner

edvin commented Mar 5, 2019

Perhaps it would make sense to have an overload for listProperty which accepts varargs as well?

@edvin
Copy link
Owner

edvin commented Mar 6, 2019

I'll do it :)

@edvin edvin merged commit 5805228 into edvin:master Mar 6, 2019
@LittleLightCz LittleLightCz deleted the functionBuildersForProperties branch March 6, 2019 17:26
@LittleLightCz
Copy link
Contributor Author

Ahh, sorry for the Changelog 😁 , but thank you ☺️

@edvin
Copy link
Owner

edvin commented Mar 6, 2019

No problem, happy for every contribution :)

@LittleLightCz
Copy link
Contributor Author

I am testing the latest snapshot version and there is one thing that hit my eye. For example now when I use stringProperty(""), then the inferred type is SimpleStringProperty. Therefore if I pass this val to some function, IDEA auto-generates parameter with SimpleStringProperty type as well, which pollutes the type name with the "Simple" word again :-)). So I was thinking, is it a good idea for the xxxProperty methods we've just added to return their parent class type instead? Could it cause any issues for some use-cases?

@edvin
Copy link
Owner

edvin commented Mar 9, 2019

It's most definitely better if they return the parent class :)

@LittleLightCz
Copy link
Contributor Author

Ok thanks for advice :-). I will do another PR and will try to fix it.

@edvin
Copy link
Owner

edvin commented Mar 9, 2019

Great, thank you!

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

Successfully merging this pull request may close these issues.

2 participants