Skip to content
This repository was archived by the owner on Jun 15, 2023. It is now read-only.

Tweaks#46

Merged
garyb merged 8 commits intoslamdata:masterfrom
garyb:tweaks
Jul 27, 2018
Merged

Tweaks#46
garyb merged 8 commits intoslamdata:masterfrom
garyb:tweaks

Conversation

@garyb
Copy link
Copy Markdown
Member

@garyb garyb commented Jul 26, 2018

A bunch of things @kritzcreek and I encountered while working on SQL generation that could be tweaked for a better experience.

@garyb garyb changed the title [WIP] Tweaks Tweaks Jul 26, 2018
@garyb garyb requested review from cryogenian and kritzcreek July 26, 2018 22:54
Copy link
Copy Markdown

@kritzcreek kritzcreek left a comment

Choose a reason for hiding this comment

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

Looks good, just seems like one typo

path ← Pt.parseAnyDirPath P.fail s
pure $ Sig.Import path

variable ∷ ∀ m t. SqlParser' m t
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Was this change intended?

@garyb
Copy link
Copy Markdown
Member Author

garyb commented Jul 27, 2018

Oops! Should be good now.

Copy link
Copy Markdown
Member

@cryogenian cryogenian left a comment

Choose a reason for hiding this comment

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

Non-blocking but good to have comment about moving genIdent :)

genImport = map Import Pt.genAnyDirPath

genIdent ∷ ∀ m. Gen.MonadGen m ⇒ m String
genIdent ∷ ∀ m. Gen.MonadGen m ⇒ m ID.Ident
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This should live in SqlSquared.Signature.Ident I think.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Makes sense 👍

Copy link
Copy Markdown
Member

@cryogenian cryogenian left a comment

Choose a reason for hiding this comment

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

Thanks!

@garyb garyb merged commit 9095d45 into slamdata:master Jul 27, 2018
@garyb garyb deleted the tweaks branch July 27, 2018 10:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants