You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@daffodil.apache.org by GitBox <gi...@apache.org> on 2023/01/16 20:35:23 UTC
[GitHub] [daffodil-vscode] scholarsmate opened a new pull request, #423: WIP: initial integration of the new data editor
scholarsmate opened a new pull request, #423:
URL: https://github.com/apache/daffodil-vscode/pull/423
**This is a work in progress**
New data editor webview extension.
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil-vscode] scholarsmate commented on pull request #423: WIP: initial integration of the new data editor
Posted by GitBox <gi...@apache.org>.
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil-vscode] stevedlawrence commented on pull request #423: WIP: initial integration of the new data editor
Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on PR #423:
URL: https://github.com/apache/daffodil-vscode/pull/423#issuecomment-1386013121
I haven't reviewed anything yet, but some high level question:
1. svlete seems to be a gui framework, does VS Code not provide a gui system that uses VS Code native widgets and things?
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?
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.
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.
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil-vscode] scholarsmate closed pull request #423: WIP: initial integration of the new data editor
Posted by "scholarsmate (via GitHub)" <gi...@apache.org>.
scholarsmate closed pull request #423: WIP: initial integration of the new data editor
URL: https://github.com/apache/daffodil-vscode/pull/423
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil-vscode] scholarsmate commented on pull request #423: WIP: initial integration of the new data editor
Posted by "scholarsmate (via GitHub)" <gi...@apache.org>.
scholarsmate commented on PR #423:
URL: https://github.com/apache/daffodil-vscode/pull/423#issuecomment-1416026590
Closing the draft PR because it's spamming emails to some folks who are not interested in following the activity on this branch.
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org