v2.1.0 -- swapped out yaml package #39
Conversation
|
Thanks for the patch! Have you run the test suite with this patch? |
CHANGELOG.md
Outdated
| @@ -1,3 +1,8 @@ | |||
| 2.1.0 | |||
There was a problem hiding this comment.
Please remove any changes to the package version. That only gets decided at release time, when all the changes in the release need to be examined together to determine the next version.
| @@ -104,8 +106,8 @@ migrationFromPath path = do | |||
| readMigrationFile = do | |||
| ymlExists <- doesFileExist (addNewMigrationExtension path) | |||
| if ymlExists | |||
| then parseYamlFile (addNewMigrationExtension path) `catch` (\(e::IOException) -> throwFS $ show e) | |||
| else parseYamlFile (addMigrationExtension path filenameExtensionTxt) `catch` (\(e::IOException) -> throwFS $ show e) | |||
| then decodeFileThrow (addNewMigrationExtension path) `catch` (\(e::IOException) -> throwFS $ show e) | |||
There was a problem hiding this comment.
The old code caught IOException. Is that how the new Yaml parser behaves as well?
There was a problem hiding this comment.
On invalid YAML it throws ParseException. I don't know what the old parser did on invalid YAML as I never got it to work. Assuming it threw IOException on invalid YAML and this needs to be caught and rethrown using throwFS I'm adding a catch for ParseException. As for actual IO errors, e.g. file permissions problems, the new parser seems to throw ParseException too so catching IOException is probably not needed at all.
hankatlas
force-pushed
the
feature/yaml_update
branch
2 times, most recently
from
f83bc05 to
a41456c
Compare
hankatlas
force-pushed
the
feature/yaml_update
branch
from
a41456c to
2fa9442
Compare
I have and I amended the wording of an error message in one test to make it pass. Thanks |
|
Okay, thanks for addressing my comments! This looks good to me and the tests pass, so I'll merge it. Thanks for your work on this! I also want to mention that it would be helpful (in the future) if you can push incremental changes to your PR rather than force-pushing. Force-pushing makes it more difficult for me to evaluate the changes as they come in, and it can lead to problems if I've pulled your branch in the mean time to do testing (which is likely). |
|
This patch is now released in 2.1.0. |
yaml-light's dependency HsSyck would compile for me because of the underlying C code not being C99 compliant. HsSyck hasn't been maintained for 8 years so instead I propose to swap outdbmigrationsYAML parsing toyaml.