Closed blackheaven closed 4 months ago
@frasertweedale I think the failure is expected as the file is oob (not tracked in Git history)
@frasertweedale FYI, I have revert back to toml-parser
and properly tested FrontMatter
rendering
@frasertweedale can you review it again, so I can move forward (see items in the description)
I won't lie, it was one of the most painful rebase I had to do.
@frasertweedale good luck :)
Seven hour train ride on Wednesday. Should be enough time, I hope :)
Why so much reformatting? :sob:
@blackheaven did you reformat using some formatter? If so, it would be better to tackle that as the first commit, and keep the signal-to-noise ratio high for the subsequent commits.
Also, should we consolidate around a particular formatting tool? (I'm not really a fan, but I'm even more not a fan of what happened in e72abb5ac60f6b3230054c12b40433ecec648d28 ^_^)
(I would advise fourmolu, as it is diff-friendly and reduces the noise of each commit)
@blackheaven some of the error formatting undoes Mango's Exception instance work. I can fix it up if you like.
Actually it was a mistake, @TristanCacqueray enforced fourmolu, but hls does not take le configuration file in account when formatting.
I'm afraid that fixing reformatting during rebase might be a nightmare.
@frasertweedale either you have some time to revert back Mango's work, or highlight where I have overwritten them.
@frasertweedale either you have some time to revert back Mango's work, or highlight where I have overwritten them.
Yes, I'll make the changes and then we can compare and review. Expect it tomorrow. Apart from that, there was just one comment so far. I've read most of the diffs and the new code LGTM at a high level. Still need to test, also.
@frasertweedale either you have some time to revert back Mango's work, or highlight where I have overwritten them.
Yes, I'll make the changes and then we can compare and review. Expect it tomorrow. Apart from that, there was just one comment so far. I've read most of the diffs and the new code LGTM at a high level. Still need to test, also.
I see that later commits in the series put everything back the way it was. The commit history is a winding road but all is well at the destination.
I will do some testing. Unless I find bugs / regressions, I will merge it :)
I have done the changes (fixings root directory and using etag), let me know what you think.
@blackheaven thanks mate. I fly home to Australia tomorrow so I will have time to review it!
Have a nice flight back!
uh @blackheaven, did you push your changes? I don't see them :confused:
I forgot and it conflicts everywhere, I'll rebase it (again)
Here we go
I've reviewed and tested it. Works as expected. There are some improvements we can make but no show-stoppers. Sorry for the delay and thanks for your patience!
I pushed one new commit that adds more detail to the HTTP error messages. I'll merge when green.
CI is busted. Dealing with it in PR https://github.com/haskell/security-advisories/pull/220.
Finally merged. Thanks for implementing this feature, @blackheaven!
hsec-tools
TBD