-
Notifications
You must be signed in to change notification settings - Fork 15
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
Code Style: Initialize Objects with Properties #143
base: master
Are you sure you want to change the base?
Code Style: Initialize Objects with Properties #143
Conversation
It works, but I would really like to get feedback on the implementation of I am merging the single texts from |
I would be really happy about a review 😃️ |
As I understand it, this looks for a pair of lines in the form
If so, you could examine pairs of lines instead of the whole contents using two Regexes. If the first of the pair matches the first Regex then test whether the second line matches the second Regex. It would make the Regexes a bit simpler but whether it is a worthwhile change I am not sure. Neither method will pick up cases where the form |
Unfortunately, I'm not sure who may be a good person to review this change, and I don't have the knowledge here to provide any good feedback. @danrabbit, do you have any advice on who may be a good person to review this pr? |
Did a quick test of this against some old code of mine, and while it picked up a lot, it misses cases where the instantiation doesn't occur in the same line as the declaration. For example, the following is not picked up: public class MyTest {
private Gtk.Button button;
construct {
button = new Gtk.Button.with_label ("Push me");
button.valign = Gtk.Align.CENTER;
}
} I wonder if the Another valid case to consider would be the use of the prefix |
I guess more tests need to be added to cover the missed cases mentioned above and more work put into the code to fail these cases. |
Closes #142