libwww-perl / WWW-Mechanize

Handy web browsing in a Perl object
https://metacpan.org/pod/WWW::Mechanize
Other
68 stars 52 forks source link

perltidy and enable tidy tests for t, pm and scripts #358

Closed simbabque closed 1 year ago

simbabque commented 1 year ago

This is how it works: What is required to be tidy is controlled by tidyall.ini. Anything listed in the perltidy section will be tested for tidiness. This happens after dzil has made changes to our files. The ignored files come from the documentation of Dist-Zilla-Plugin-Test-TidyAll.

simbabque commented 1 year ago

The tests are currently failing because Pod::Weaver leaves behind blank lines when it moves POD blocks from in between functions. I don't know how to fix that, but I think the easiest way will be to make that part of the perltidyrc so we actually require it. Although that might lead to having two extra lines.

@@ -1966,7 +1894,6 @@ sub _push_page_stack {
     return 1;
 }

-
 sub warn {

I compared Mechanize.pm from within the .build folder before and after manual tidying. See above for what is happening.

oalders commented 1 year ago

This has a merge conflict as well now.

simbabque commented 1 year ago

I've resolved the conflict. Looking at Pod::Weaver next.

oalders commented 1 year ago

Looks like there are new merge conflicts. 😄

simbabque commented 1 year ago

I couldn't figure out a solution for the Pod::Weaver issue. It definitely needs more work now because other stuff is coming in again.

oalders commented 1 year ago

For the Pod::Weaver stuff, that shouldn't be an issue if we only run tidyall on files in the repo rather than generated files?

simbabque commented 1 year ago

For the Pod::Weaver stuff, that shouldn't be an issue if we only run tidyall on files in the repo rather than generated files?

I am not sure that would work. The way I understand the tests, it builds the dist first, and then runs the xt test that Dist-Zilla-Plugin-Test-TidyAll has made in the dist folder, which of course operates on the built dist. At that point PodWeaver is done, and it checks the final .pm files that have had their POD woven.

How would you do it?

oalders commented 1 year ago

Right. That's kind of a tough one to solve. We could maybe skip that test if it's not run inside of a repo or we could remove the blank lines before running the test. Neither seem very attractive.

simbabque commented 1 year ago

I've solved it by allowing two empty lines in the perltidyrc file. I ran it past @haarg at the PTC, so this should now be ready to land pending review.

haarg commented 1 year ago

This adds a file t/Tools.pm that doesn't seem to be used.

simbabque commented 1 year ago

This adds a file t/Tools.pm that doesn't seem to be used.

Sorry, that was removed in one of the commits I rebased on top of, and I added it back in instead of taking the deletion. Fixed now.