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

Partial string search support #43

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Conversation

mithyalabs
Copy link
Contributor

  • Added contains operator for partial string match in queries.

@mithyalabs mithyalabs requested a review from adlerfaulkner May 25, 2024 12:16
Copy link
Contributor

@adlerfaulkner adlerfaulkner 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 please separate the auto formatting changes from the meaningful code changes here? It makes it pretty time intensive to review when they're combined

@@ -93,4 +93,4 @@
"peerDependencies": {
"axios": "^0.27.2"
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Can you re-add the newline at the end of this file please?

import { Mapper } from './mapping/Mapper';
import type { SklEngineOptions } from './SklEngineOptions';
import type { FindOperator } from './storage/FindOperator';
import type { FindAllOptions, FindOneOptions, FindOptionsWhere } from './storage/FindOptionsTypes';
import type {
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for cleaning up these imports!

@@ -55,11 +66,13 @@ import { SKL, SHACL, RDFS, SKL_ENGINE, XSD, RDF } from './util/Vocabularies';

export type VerbHandler = <T extends OrArray<NodeObject> = OrArray<NodeObject>>(
params: JSONObject,
verbConfig?: VerbConfig,
verbConfig?: VerbConfig
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: We prefer the trailing comma here

@@ -0,0 +1,21 @@
export class Logger {
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for adding this. We can make it better over time for example by adding winston.

Looks like there are a couple linter items here. You can eslint ignore the one about console.log

@adlerfaulkner
Copy link
Contributor

Please rebase with main. I added husky to the prepare command so that it will install itself for you

@mithyalabs mithyalabs requested a review from adlerfaulkner June 4, 2024 15:46
@adlerfaulkner
Copy link
Contributor

There are still tons of auto-formatting changes in src/SklEngine.ts, can you revert those please?

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