Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#496] Run Stan on Summoner and fix observations #498

Merged
merged 3 commits into from
Aug 9, 2020

Conversation

vrom911
Copy link
Member

@vrom911 vrom911 commented Aug 8, 2020

Resolves #496

There were 30 observations, now down to 2 (length ones which are not resolvable now).
The code was actually pretty good already 👍🏼

Copy link
Contributor

@hint-man hint-man bot left a comment

Choose a reason for hiding this comment

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

There is no place for me here... I will choose the truth I like.

Copy link
Contributor

@chshersh chshersh left a comment

Choose a reason for hiding this comment

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

Looking good! I love how Stan helped us here to make Summoner better ⚕️
But turned out, as you said, Summoner already was good enough 👍

summoner-cli/src/Summoner/GhcVer.hs Outdated Show resolved Hide resolved
Comment on lines +114 to 115
upperBound ghcs = let Pvp{..} = baseVerPvp $ last ghcs in
show pvpFirst <> "." <> show (pvpSecond + 1)
Copy link
Contributor

Choose a reason for hiding this comment

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

Very nice refactoring! No partial functions in the end 😎

Comment on lines -39 to -41
where
isSuccess :: Validation e a -> Bool
isSuccess = \case { Success _ -> True; _ -> False }
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice!

Comment on lines +233 to +235
validatedErrorMessages = whenFailure []
(validateKit dirs $ formState kitForm)
(map showFormError . toList)
Copy link
Contributor

Choose a reason for hiding this comment

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

Turns out, we implemented useful functions in validation!

Co-authored-by: Dmitrii Kovanikov <kovanikov@gmail.com>
@chshersh chshersh merged commit d2d9497 into master Aug 9, 2020
@chshersh chshersh deleted the vrom911/496-Run-Stan-on-Summoner-and branch August 9, 2020 09:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Run Stan on Summoner and fix observations
2 participants