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

Add overload that allows a custom parameter placeholder. #3622

Merged
merged 5 commits into from
Jan 24, 2025

Conversation

elexisvenator
Copy link
Contributor

@elexisvenator elexisvenator commented Jan 10, 2025

The char placeholder param is always before the string sql param. This is to ensure that the overload is backwards compatible.,

Updated all methods that accept parameters except for:

  • Sync methods that are marked obsolete
  • MatchesJsonPath - This should probably be marked as obsolete as the placeholder overload does the same thing but better.

Todo:

  • Tests.
  • Documentation

The `char placeholder` param is always before the `string sql` param. This is to ensure that the overload is backwards compatible.,

Updated all methods that accept parameters except for:
- Sync methods that are marked obsolete
- `MatchesJsonPath` - This should probably be marked as obsolete as the placeholder overload does the same thing but better.

Todo:
- Tests.
@@ -62,6 +114,7 @@ public interface IAdvancedSql
/// <param name="sql"></param>
/// <param name="parameters"></param>
/// <returns></returns>
[Obsolete(QuerySession.SynchronousRemoval)]
IReadOnlyList<T> Query<T>(string sql, params object[] parameters);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A couple of spots where the obsolete warning for synchronous db methods was missing.

@elexisvenator elexisvenator marked this pull request as ready for review January 13, 2025 11:22
@elexisvenator elexisvenator marked this pull request as draft January 13, 2025 11:22
@elexisvenator
Copy link
Contributor Author

There will be some small merge conflicts between this and #3604. Some of the wording in the documentation will need to change. There should be no issues with the code itself.

@elexisvenator elexisvenator marked this pull request as ready for review January 23, 2025 05:39
@elexisvenator
Copy link
Contributor Author

failing tests are related to stronglyTypedIds and don't appear to be related to these code changes

@jeremydmiller jeremydmiller merged commit e8524f6 into JasperFx:master Jan 24, 2025
2 of 5 checks passed
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