-
Notifications
You must be signed in to change notification settings - Fork 2
Initialize repository with tree component #1
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
Initialize repository with tree component #1
Conversation
jreineckearm
left a comment
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.
Awesome, thanks @haydar-metin !
Haven't tested yet. Will give it a try with the Peripheral Inspector branch.
Also need to have another look at how the parts play together to fully understand the code.
Just to confirm: this is for the start only the tree-table, right? It does not include the simpler tree variant with inline editing/controls.
We may want to set something up around TPIP tracking. But I believe this is something that also affects other repos.
Yes, I only moved the components from the other repository. Therefore, there are no changes. I will update the components again after the open PR with editing capabilities has been merged. |
jreineckearm
left a comment
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.
Some more review comments. It would be great to give developers more guidance on how to reuse existing TreeDataProviders in the README.md.
Tested locally with suggested peripheral inspector PR. What I tested worked nicely. :-)
Overall, this looks great. Happy to approve after feedback has been addressed.
Awesome work on this!
|
@jreineckearm Thanks for the feedback! Please see 31746ed for the changes. Publishing to NPM will be done in a separate PR. |
da752c4 to
31746ed
Compare
jreineckearm
left a comment
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.
Awesome! Thanks, @haydar-metin , for incorporating this feedback!
Just a few minor comments/questions. But I think we can merge after addressing/answering those.
Let's bring in pending changes from open Peripheral Inspector changes as they come.
Also, let's look at TPIP docs separately as mentioned in the comment.
jreineckearm
left a comment
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.
Awesome, thanks! Good to merge.
Provides an initial structure for the repository:
Project
Used Tools:
Components:
Structure:
The components are extracted from https://github.com/eclipse-cdt-cloud/vscode-peripheral-inspector
How to test
See eclipse-cdt-cloud/vscode-peripheral-inspector#51