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

feat: probe function #17

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

feat: probe function #17

wants to merge 2 commits into from

Conversation

Lukejkw
Copy link

@Lukejkw Lukejkw commented Jul 3, 2023

Opening PR for comment to see if this is something that would be useful and feedback.

A probe function is essentially like a bisection - allowing you to "probe" into some data to see what a value is without actually changing the output. It's usually useful for debugging purposes.

Copy link
Owner

@ashleydavis ashleydavis left a comment

Choose a reason for hiding this comment

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

Hey @Lukejkw thanks so much. Just a couple of minor changes to make and one major change. I've left comments in the code for you to look at.

Cheers
Ash

const { fn, details } = loadUserFn(argv, `record => transform(record)`);

const data = await inputData(argv);
verifyInputArray(data, 'map');
Copy link
Owner

Choose a reason for hiding this comment

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

This should take the name of the command to display for the help, which is 'probe' instead of 'map'.

const data = await inputData(argv);
verifyInputArray(data, 'map');

data.map((record: any) => {
Copy link
Owner

Choose a reason for hiding this comment

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

Can we change this to a forEach because it doesn't actually have to create a new array.


data.map((record: any) => {
const result = invokeUserFn(() => fn(record), details);
console.log(result);
Copy link
Owner

Choose a reason for hiding this comment

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

@Lukejkw I just realised this isn't going to work.

Because Datakit relies on being able to output clean JSON data to the stdout you can't get away with console logging anything like this.

You could change it to console.error. Would that work for you?

data.map((record: any) => {
const result = invokeUserFn(() => fn(record), details);
console.log(result);
return record;
Copy link
Owner

Choose a reason for hiding this comment

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

Once the map above is changed to forEach this return statement can be removed.

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