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

use sqlparse position tracking #4

Merged
merged 3 commits into from
Sep 28, 2023
Merged

Conversation

dylanscott
Copy link

@dylanscott dylanscott commented Sep 21, 2023

This PR pulls in the version of sqlparse from hex-inc/sqlparse#4 and reverts the old position tracking implementation that we added and replaces it with a much simpler version that uses the token positions and lengths now provided by sqlparse. I'm doing some dev work on the ast-service with these new versions pulled in and have confirmed that they do not regress any tests

@dylanscott dylanscott changed the title use sqlparse positions use sqlparse position tracking Sep 21, 2023
@dylanscott dylanscott requested a review from jkillian September 21, 2023 21:53
Copy link

@jkillian jkillian left a comment

Choose a reason for hiding this comment

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

Nice, like that this gets us back closer to the original!

pyproject.toml Outdated
@@ -1,24 +1,26 @@
[tool.poetry]
name = "sql_metadata"
version = "2.8.0"
license="MIT"
license = "MIT"

Choose a reason for hiding this comment

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

All these formatting changes in this file seem like changes we might not want if it makes it differ from upstream?

Copy link
Author

Choose a reason for hiding this comment

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

sorry I think this was an overzealous vs code toml formatter - will revert the formatting changes

Base automatically changed from dscott/sync-fork to master September 28, 2023 16:19
@dylanscott dylanscott force-pushed the dscott/use-sqlparse-positions branch from 6a55a75 to 10c065a Compare September 28, 2023 16:45
@github-actions
Copy link

Pull Request Test Coverage Report for Build 6341838340

  • 13 of 13 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 6341341855: 0.0%
Covered Lines: 872
Relevant Lines: 872

💛 - Coveralls

@dylanscott dylanscott merged commit f038ac5 into master Sep 28, 2023
6 checks passed
@dylanscott dylanscott deleted the dscott/use-sqlparse-positions branch September 28, 2023 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants