-
Notifications
You must be signed in to change notification settings - Fork 8
Add perltidy to lint workflow #59
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
Add perltidy to lint workflow #59
Conversation
|
Please add any comments/discussion for changes to the .perltidyrc file. We want this to be set before this is merged. We definitely don't want this to change much. Every time that is done the entire code base will need to have perltidy run on, and that will get messy. |
|
@drdrew42: I looked into the At this point the only difference in our code is in lib/DB/Utils.pm starting on line 76. With the $fields_to_return->{$key} =
defined($updated_fields->{$key}) ? $updated_fields->{$key} : $current_fields->{$key};With the $fields_to_return->{$key} =
defined($updated_fields->{$key})
? $updated_fields->{$key}
: $current_fields->{$key};Note that if you make the ternary statement longer than 120 characters then both rules make it look like the second case. If the statement is short enough to fit onto the same line as the equality, then both rules will let that happen. However, in this case, |
|
Also, note that with my example above (of the only place the rule makes a difference in our code), if you write the code the way that perltidy changes it to with the |
|
Overall, this looks good and didn't change too much. About the ternary statements, I would go with the |
|
@pstaapb: Is there any reason why you don't like the |
|
I don't feel that strongly either way. |
|
Actually, I don't feel strongly one way or the other on the |
|
I'm okay with |
The first commit adds perltidy to the workflow.
The second commit just runs perltidy on the code base.
Note that you need the latest version of perltidy from cpan for this (20210717). The Ubuntu version 20190601-1 does not work the same.