scholarsmate commented on PR #423: URL: https://github.com/apache/daffodil-vscode/pull/423#issuecomment-1386099424
Thanks @stevedlawrence, my replies are inline. > 1. svlete seems to be a gui framework, does VS Code not provide a gui system that uses VS Code native widgets and things? Svelte is MIT licensed _compiler_ and allows us to write reactive components without any runtime dependencies as it compiles these components into javascript. Notice that all the svelte stuff are development dependencies. This is in stark contrast to something like react.js which has runtime dependencies. > 2. Why is some svelte stuff in a subdirectory/project with it's own package dependencies and sources? How are those incorporated into the main project? First question: The svelte directory contains the source to generate components that are then consumed by the extension. The build tools are installed via a post install hook and the components are compiled via a pre compile hook. The built artifacts we need at runtime will be in `svelte/dist`. Second question: The svelte components get compiled into javascript that goes into the `svelte/dist` directory. We have one component in there now, the whole Data Editor, so svelte compiles it into a single stylesheet (`styles.css`) and one component script (`views/dataEditor/index.js`). This component is used as a webview for the data editor piece of the extension. > 3. Have you verified all the svelte dependencies are ALv2 friendly? Seems like a lot of new dependencies are being added, but maybe it's not as much as it seems. Svelte, sass, and rollup are MIT. We ship none of those. There are only 2 runtime dependencies which are VS Code icons and HTML UI elements. > 4. This PR closes 12 issues, can they not be separated into multiple PR's to make the reviewers lives happier? It's so much easier to review multiple small patches. Please try avoid doing a ton of work behind the scenes and pushing it for review when it's all done. Incremental enhancements are sooooo much better. I agree with this in general, but I believe this kind of thing is an exception. The majority of the issues that all get closed by this PR are requirements that this PR is designed to meet. This PR is going to take a while to bake (a few more weeks), thus the draft state, but I wanted to get this out for some incremental review as we go (case in point). Breaking it up into pieces, one for each issue in this case would lead to merging a bunch of ingredients and half-baked building blocks. It would also take _forever_. Like merging bread, meat, lettuce, cheese, and condiments to build a sandwich as part of a larger lunch. We need to complete the sandwich in this branch, then we can make incremental improvements once a functional minimal viable feature is completed, reviewed, and merged. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
