Skip to content
This repository was archived by the owner on Apr 5, 2024. It is now read-only.

Upgrade lts - #26

Open
axman6 wants to merge 9 commits into
masterfrom
feature/upgrade-lts
Open

Upgrade lts#26
axman6 wants to merge 9 commits into
masterfrom
feature/upgrade-lts

Conversation

@axman6

@axman6 axman6 commented May 4, 2020

Copy link
Copy Markdown

Welcome to the 21st century, evaporate! Most of the changes come from applying the stylish-haskell config we use for other internal projects to the whole project. All the function changes are to make the project compile with new LTSs.

The only thing I'm unsure about is the change to throwing instead of using fail

- Failure _ -> fail . unpack $ t <> " is not a valid value"
+ Failure _ -> throwM $ InvalidBucketName $ t <> " is not a valid value"

I couldn't tell why it was using fail, and there isn't an instance for MonaFail for AWST.

@axman6
axman6 requested review from askoufis and tmortiboy May 4, 2020 04:10

@askoufis askoufis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing this! Just a bit of background, I wrote this with the help of the autoinc team while I was at SEEK for my industry placement, and it was my first foray into haskell, and I haven't really done much haskell since then, so I don't really have any strong opinions on how stuff should work, so feel free to upgrade/tweak things how you see fit.

Comment thread test/LoggingSpec.hs
stackParams <- runExceptT . getStackParameters $ "test/valid.yaml"
let descriptions = fromRight' stackParams
let accountID = "478156153062"
let accountID = "478156153XXX"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this a real account ID? 😱 🤦

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No idea, it's been in there for a long time so hopefully it's all good. I only came across it because a recommit hook fired and said is was no es bueno. 😬

Comment thread build-publish.dockerfile Outdated
@@ -1,9 +1,9 @@
FROM ubuntu:14.04
FROM ubuntu:16.04

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

18.04 is even smaller. Hesitant to go to 20.04 though, too new.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah perhaps we should give it a go, we've avoided moving off 16.04 but here it would probably be fine.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants