search

eivindveg / HotSUploader

JavaFX-based Replay Uploader for Heroes of the Storm
Apache License 2.0
185 stars 36 forks source link

Use Files#walk as directory travsersal #151

Closed Tristan971 closed 7 years ago

Tristan971 commented 7 years ago

Hello,

A pretty explicit commit. Using Files#walk allows for leverage of the streams api and the use of jdk api rather than custom implementation. I chose to throw a RuntimeException mostly because it seems to be the policy of the project to prefer that to failing silently. That said it could be interesting to log errors and return a Collections.emptyList() instead.

This is mostly a matter of opinion though.

Regards

eivindveg commented 7 years ago

Great job. On the note of RuntimeExceptions versus "silent failures", the reasoning so far has been that we have no method of indicating to the user that something has gone wrong and that a bug report should be submitted. We therefore "force" a crash instead, and plan to resolve this in the medium term.

eivindveg commented 7 years ago

Retriggered Travis build. That race condition in the test really bugs me.

Tristan971 commented 7 years ago

Yes that was pretty weird... I was quite surprised.

Anyway thanks for taking the time to review these PRs !