preaction / Statocles

Static website CMS
http://preaction.me/statocles
Other
84 stars 33 forks source link

Tweaks #558

Closed mohawk2 closed 6 years ago

mohawk2 commented 6 years ago

These are the independent changes I've felt necessary while fighting to separate the paths that are file-paths from paths that are parts of URLs :-)

Notably, I added a param to the App's url method to control it stripping off index.html. I thought that was worthwhile because of the useful extra-/-avoiding behaviour it has, which is important for Mojo::Path objects, while Path::Tiny just hides them.

I hope you also find the test_pages behaviour change useful - now if there are missing or extra pages it will tell you just which ones, not all of both.

coveralls commented 6 years ago

Coverage Status

Coverage increased (+0.006%) to 91.93% when pulling 87074d85d529ef4f14171a829ca6eeb1d03bcd8b on mohawk2:tweaks into 2f9b8f525ac952e9c54b07488a272bcb8e6b77dc on preaction:master.

preaction commented 6 years ago

This looks great: Centralizing the creation of paths/URLs on the Site object is the right thing to do, and the test_pages change is long overdue.

Is this ready for merge, or do you want to keep working on it?

mohawk2 commented 6 years ago

I feel this is ready for merge as-is! I've just got the switch to Mojo::Path working as in passing all tests, but it's still doing some Path::Tiny stuff that's failing on Windows so a little more is required :-)

mohawk2 commented 6 years ago

(To be clear, that stuff I'm doing on a separate branch, which you can look at if you're interested: https://github.com/mohawk2/Statocles/commits/type-pagepath )

preaction commented 6 years ago

Okay, I'll put this in and see if I can start cleaning up the existing tests a bit according to my new plan of reducing the scope of individual test units (everything is an integration test now, and I'm sure you've noticed that the whole thing takes for-ever to run).