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

chore: sql property parsing io sql package #490

Merged
merged 7 commits into from
Dec 9, 2022
Merged

chore: sql property parsing io sql package #490

merged 7 commits into from
Dec 9, 2022

Conversation

stijnmoreels
Copy link
Member

Use our own version of parsing SQL properties instead of relying on a SQL package.

Closes #488

@netlify
Copy link

netlify bot commented Nov 30, 2022 •
edited
Loading

✅ Deploy Preview for arcus-observability canceled.

Name Link
🔨 Latest commit d4aa1bb
🔍 Latest deploy log https://app.netlify.com/sites/arcus-observability/deploys/6392d2d32d4b3c0008a98136

@stijnmoreels
Copy link
Member Author

This also means that we have no use anymore for a dedicated package and we should slowly moving them back to the .Core package.

@fgheysels fgheysels changed the title chore: sql property parsting io sql package chore: sql property parsing io sql package Nov 30, 2022
@stijnmoreels
Copy link
Member Author

This is actually property-based testing hiding as data-driven tests; but chose to use the latter as we should make sure that starters (which do not see property-based testing, normally) could follow.

@fgheysels fgheysels assigned stijnmoreels and unassigned fgheysels Dec 6, 2022
@stijnmoreels
Copy link
Member Author

We could already move the SqlConnectionStringParser and its result to the .Core package, so that our later migration is only about deprecating the extensions in the .Sql package and adding copies to the .Core package. Ok?

@fgheysels fgheysels assigned stijnmoreels and unassigned fgheysels Dec 8, 2022
@fgheysels fgheysels assigned stijnmoreels and unassigned fgheysels Dec 9, 2022
@stijnmoreels stijnmoreels merged commit 9508695 into arcus-azure:master Dec 9, 2022
@stijnmoreels stijnmoreels deleted the chore/sql-property-parsing branch December 9, 2022 11:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fgheysels fgheysels fgheysels approved these changes

Assignees

@stijnmoreels stijnmoreels

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Can we remove the dependency to Microsoft.Data.SqlClient in Arcus.Observability.Telemetry.Sql
2 participants
@stijnmoreels @fgheysels