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.

Sign up for GitHub

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

Jump to bottom

Use a SqlStatement within template literal #52

Merged
merged 21 commits into from
May 7, 2021
Merged

Use a SqlStatement within template literal #52

merged 21 commits into from
May 7, 2021

Conversation

penx
Copy link
Contributor

@penx penx commented May 6, 2021 •
edited
Loading

  • Use a SqlStatement within template literal, e.g. the output of SQL.glue or another use of SQL``
  • Deprecate .append in favour of nested template literals
  • Clarifications that generateString is not part of the public API

Fixes #51

penx
penx commented May 6, 2021
View reviewed changes
SQL.js Outdated Show resolved Hide resolved
@penx penx mentioned this pull request May 6, 2021
Closed
penx
penx commented May 6, 2021
View reviewed changes
SQL.js Outdated Show resolved Hide resolved
penx
penx commented May 6, 2021
View reviewed changes
SQL.test.js Show resolved Hide resolved
@penx penx requested a review from simoneb May 6, 2021 17:32
penx
penx commented May 6, 2021
View reviewed changes
SQL.js Outdated Show resolved Hide resolved
@penx penx added the enhancement New feature or request label May 6, 2021
simoneb
simoneb reviewed May 7, 2021
View reviewed changes
SQL.js Outdated Show resolved Hide resolved
SQL.js Outdated Show resolved Hide resolved
@penx penx requested a review from simoneb May 7, 2021 09:38
@penx
Copy link
Contributor Author

penx commented May 7, 2021

I'm taking this out of draft, though documentation is still needed

@penx penx marked this pull request as ready for review May 7, 2021 09:47
simoneb
simoneb reviewed May 7, 2021
View reviewed changes
Copy link
Member

@simoneb simoneb left a comment

Choose a reason for hiding this comment

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

1 inline comment, add docs please. Also, shall we deprecate .append then? With this feature and the earlier unsafe function, .append becomes pretty much redundant.

SQL.js Outdated Show resolved Hide resolved
mcollina
mcollina approved these changes May 7, 2021
View reviewed changes
Copy link
Contributor

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

mcollina
mcollina reviewed May 7, 2021
View reviewed changes
Copy link
Contributor

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

Can you document this in the readme?

Alasdair McLeay added 2 commits May 7, 2021 11:42
deprecate append with jsdoc in code
40cf3e9
Update readme
6f45a53 
to not use append in code examples and deprecate append
@penx penx requested a review from simoneb May 7, 2021 10:44
mcollina
mcollina approved these changes May 7, 2021
View reviewed changes
Copy link
Contributor

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

mcollina
mcollina reviewed May 7, 2021
View reviewed changes
SQL.js Outdated Show resolved Hide resolved
@penx penx requested a review from mcollina May 7, 2021 11:14
penx
penx commented May 7, 2021
View reviewed changes
SQL.js Outdated Show resolved Hide resolved
mcollina
mcollina approved these changes May 7, 2021
View reviewed changes
Copy link
Contributor

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

simoneb
simoneb approved these changes May 7, 2021
View reviewed changes
Copy link
Member

@simoneb simoneb left a comment

Choose a reason for hiding this comment

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

Very nice!

@penx penx changed the title Use SQL.glue from within template literal Use a SqlStatement within template literal May 7, 2021
@penx penx merged commit 584a196 into master May 7, 2021
@penx penx deleted the feature-51-glue-within-literal branch May 7, 2021 13:29
@github-actions github-actions bot mentioned this pull request May 15, 2021
Closed
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@simoneb simoneb simoneb approved these changes

@mcollina mcollina mcollina approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Using SQL.glue within template literal
3 participants
@penx @simoneb @mcollina