google / blockly-samples

Plugins, codelabs, and examples related to the Blockly library.
http://github.com/google/blockly
Apache License 2.0
836 stars 618 forks source link

Move captureWarnings to dev-tools plugin #567

Closed alschmiedt closed 3 years ago

alschmiedt commented 3 years ago

Category plugins

Component dev-tools

Is your feature request related to a problem? Please describe.

Describe the solution you'd like Move captureWarnings helper from navigation_modify_test.mocha.js to dev-tools

Describe alternatives you've considered

Additional context

ricknjacky commented 3 years ago

I'd like to work on this issue. Just wanted to ask, all there is to do is move this captureWarnings function to plugins/dev-tools/test/index.js right?

image

moniika commented 3 years ago

Not exactly, it should be moved under plugins/dev-tools/src/. The dev-tools plugin provides testHelpers that are used in other plugins and in core. I'd probably add it to either the common_test_helpers.mocha.js file or create a new file like utility_test_helpers.mocha.js and also add it as an export in test_helpers.mocha.js

ricknjacky commented 3 years ago

Not exactly, it should be moved under plugins/dev-tools/src/. The dev-tools plugin provides testHelpers that are used in other plugins and in core. I'd probably add it to either the common_test_helpers.mocha.js file or create a new file like utility_test_helpers.mocha.js and also add it as an export in test_helpers.mocha.js

Thanks for the insight, I have opened a PR and did as you've told. Please share your suggestions, feedback for the same. Thanks :)

ricknjacky commented 3 years ago

After PR 1 is merged, then we need to wait for publish on npm with the changes from PR 1, and then you can test and merge PR 2

@moniika Could you please update me when the changes from my PR linked above are published on npm so that I can start working on PR2

rachel-fenichel commented 3 years ago

Edit: I just saw that the merge was a few hours ago. The next publish will be this Thursday.