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

Apply perltidy to starcheck #405

Merged
merged 1 commit into from
Mar 8, 2023
Merged

Apply perltidy to starcheck #405

merged 1 commit into from
Mar 8, 2023

Conversation

taldcroft
Copy link
Member

@taldcroft taldcroft commented Jan 9, 2023

Description

I did this: find . -name '*.p[ml]' | xargs perltidy -b -pt=2 -mft=1 -l 88 --no-valign

Interestingly, even though (in fine Perl form) perltidy has a bazillion formatting options, they really want to vertically align all the => in a hash or = in subsequent assignments. I discovered at https://stackoverflow.com/questions/4538209 that there is an undocumented --no-valign option.

Maybe the Perl style is to do all that vertical alignment, but to me it just looks old.

For the record, I installed perltidy in a dev ska env with cpanm Perl::Tidy and this worked just fine.

Interface impacts

None

Testing

Functional tests

Just regression.

Ran the standard starcheck set and saw no diffs. Ran the set we used for #402 in

https://icxc.cfa.harvard.edu/aspect/test_review_outputs/starcheck-pr405/

and see no diffs.

# Long week and IR Zone holds
starcheck -dir /data/mpcrit1/mplogs/2022/DEC2622/oflsa -out dec2622a_flight
/home/jeanconn/git/starcheck/sandbox_starcheck -dir /data/mpcrit1/mplogs/2022/DEC2622/oflsa -out dec2622a_test
/proj/sot/ska/bin/diff2html dec2622a_flight.txt dec2622a_test.txt > dec2622a_diff.html

# Monitor star
starcheck -dir /data/mpcrit1/mplogs/2022/NOV2822/oflsa -out nov2822a_flight
/home/jeanconn/git/starcheck/sandbox_starcheck -dir /data/mpcrit1/mplogs/2022/NOV2822/oflsa -out nov2822a_test
/proj/sot/ska/bin/diff2html nov2822a_flight.txt nov2822a_test.txt > nov2822a_diff.html

# Maneuver only loads
starcheck -dir /data/mpcrit1/mplogs/2022/OCT2422/oflsb -out oct2422b_flight
/home/jeanconn/git/starcheck/sandbox_starcheck -dir /data/mpcrit1/mplogs/2022/OCT2422/oflsb -out oct2422b_test
/proj/sot/ska/bin/diff2html oct2422b_flight.txt oct2422b_test.txt > oct2422b_diff.html

# Replan
starcheck -dir /data/mpcrit1/mplogs/2021/MAY0521/oflsa -out may0521a_flight
/home/jeanconn/git/starcheck/sandbox_starcheck -dir /data/mpcrit1/mplogs/2021/MAY0521/oflsa -out may0521a_test
/proj/sot/ska/bin/diff2html may0521a_flight.txt may0521a_test.txt > may0521a_diff.html

@taldcroft taldcroft mentioned this pull request Jan 9, 2023
1 task
Copy link
Contributor

@jeanconn jeanconn left a comment

Choose a reason for hiding this comment

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

I haven't checked every line but agree on the process and like the result.

@jeanconn
Copy link
Contributor

(I suppose I should have approved conditional on confirmation this actually runs (and basically does not apply any code changes), but I had some confidence in perltidy in that regard.)

@taldcroft
Copy link
Member Author

I'm not sure what guarantees are provided by perltidy about code correctness, but presumably it is just munging whitespace that isn't significant in perl.

@jeanconn - I'll run the list of regression loads from #402 and confirm no unexpected diffs and document that prior to merge.

@jeanconn jeanconn merged commit 9711ad6 into master Mar 8, 2023
@jeanconn jeanconn deleted the perltidy branch March 8, 2023 17:30
@javierggt javierggt mentioned this pull request Jul 12, 2023
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.

2 participants