-
Notifications
You must be signed in to change notification settings - Fork 6
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
DOC-672 Bloblang playground #232
Conversation
✅ Deploy Preview for docs-ui ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Msg: message, | ||
Meta: metadata, | ||
}, "", " "); err != nil { | ||
return fmt.Sprintf("Failed to marshal output: %s", err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do the fmt messages get sent out to gulp too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No. These messages are returned when the blobl()
function is executed in the browser.
golang.org/x/xerrors v0.0.0-20240903120638-7835f813f4da // indirect | ||
google.golang.org/protobuf v1.35.1 // indirect | ||
gopkg.in/natefinch/lumberjack.v2 v2.2.1 // indirect | ||
gopkg.in/yaml.v3 v3.0.1 // indirect |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
those indirect dependencies caught my attention. Not sure if it's worth it https://stackoverflow.com/questions/61115111/how-to-avoid-indirect-dependencies-in-go-mod-file
but this posts says that we can run a go mod tidy
to try to remove them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for reminding me 😄 I ran it but it just removes unused deps rather than all indirect ones.
const command = `GOOS=js GOARCH=wasm go build -o ${wasmOutput} ${wasmMain}` | ||
exec(command, (err, stdout, stderr) => { | ||
if (err) { | ||
log.error('Error building wasm:', stderr) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
check if this error output is correct. Looks weird.
gulpfile.js
Outdated
done(err) | ||
return | ||
} | ||
log('WebAssembly built successfully:', stdout) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
log level?
color: var(--body-font-color); | ||
} | ||
|
||
.button-bar { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this considers mobile? should we encompass in min-width?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested on mobile, and it is responsive. The buttons wrap.
const dataType = placeholder.getAttribute('data-type'); | ||
if (!dataType) { | ||
console.error('Data type attribute is missing on the placeholder.'); | ||
console.info('Data type attribute is missing on the placeholder.'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a recoverable error?
src/partials/bloblang-playground.hbs
Outdated
aceInputEditor = ace.edit("ace-input"); | ||
aceInputEditor.setTheme("ace/theme/github"); | ||
aceInputEditor.session.setMode("ace/mode/json"); | ||
aceInputEditor.session.setTabSize(4); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's 4? 4 tabs? 4px? Consider putting it into a FINAL_VARIABLE. If this doesn't make sense in this context, please disconsider.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spaces. Good idea
JSON.parse(str); | ||
return true; | ||
} catch (e) { | ||
return false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
log the error? possibly return the error to the user
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the result, it's not necessary. We just check so that we can format the output as JSON. If the result is not JSON, there was an error in the input or mapping, and we display that as plain text.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved. All review comments are not blockers.
UI changes required for redpanda-data/rp-connect-docs#98