Skip to content

Conversation

@nirmal25990
Copy link

@nirmal25990 nirmal25990 commented Feb 8, 2024

@chris-pardy
Copy link
Collaborator

@nirmal25990 I'd prefer if we didn't have a custom json stringifier function here.
I think we'd be better off changing the signature of the debug method to take mulitple arguments and pass them all unaltered to console.log (or console.debug honestly)
The built-in formatters already handle objects and BigInts fine so that's probably a much better option than needing the JSON.stringify here.

@chris-pardy chris-pardy self-requested a review September 30, 2024 16:38
`condition::evaluate <${jsonStringifyWithBigInt(leftHandSideValue)} ${
this.operator
} ${JSON.stringify(rightHandSideValue)}?> (${result})`
} ${jsonStringifyWithBigInt(rightHandSideValue)}?> (${result})`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's avoid a custom stringifier here when we can instead rely on the console.log built-in formatter if we change the signature of debug

chris-pardy added a commit to chris-pardy/json-rules-engine that referenced this pull request Oct 11, 2024
Change the call signature, leverage built-in formatting on console. Move debug enablement checks to file load.
Replaces CacheControl#363
Resolves CacheControl#362

Co-Author Nirmal Patel <nirmal259907@gmail.com>
chris-pardy added a commit to chris-pardy/json-rules-engine that referenced this pull request Oct 11, 2024
Change the call signature, leverage built-in formatting on console. Move debug enablement checks to file load.
Replaces CacheControl#363
Resolves CacheControl#362

Co-Author Nirmal Patel <nirmal259907@gmail.com>
@chris-pardy chris-pardy mentioned this pull request Oct 11, 2024
@chris-pardy
Copy link
Collaborator

Replaced by #378

chris-pardy added a commit to chris-pardy/json-rules-engine that referenced this pull request Oct 15, 2024
Change the call signature, leverage built-in formatting on console. Move debug enablement checks to file load.
Replaces CacheControl#363
Resolves CacheControl#362

Co-Author Nirmal Patel <nirmal259907@gmail.com>
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.

3 participants