Skip to content
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

Added detailed description about extension types #186

Merged
merged 1 commit into from
Aug 25, 2021
Merged

Conversation

JonasHelming
Copy link
Contributor

@JonasHelming JonasHelming commented Aug 2, 2021

fixed #185

Contributed on behalf of STMicroelectronics

Signed-off-by: Jonas Helming jhelming@eclipsesource.com

@JonasHelming JonasHelming changed the title WIP - Added detailled description about extenssion types Added detailled description about extenssion types Aug 2, 2021
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update the commit message, there are currently typos:

Added detailled description about extenssion types

Added detailed descriptions about extension types

I think there is still some work to be done regarding the overall content of the documentation added to make it clearer and more informative.

src/docs/extensions.md Outdated Show resolved Hide resolved
src/docs/extensions.md Show resolved Hide resolved
src/docs/extensions.md Show resolved Hide resolved
src/docs/extensions.md Outdated Show resolved Hide resolved
src/docs/extensions.md Outdated Show resolved Hide resolved
src/docs/extensions.md Outdated Show resolved Hide resolved
src/docs/extensions.md Outdated Show resolved Hide resolved
src/docs/extensions.md Outdated Show resolved Hide resolved
src/docs/extensions.md Outdated Show resolved Hide resolved
src/docs/extensions.md Outdated Show resolved Hide resolved
Copy link

@alvsan09 alvsan09 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be great if the new image for extensions depicts:

  • How @Theia extensions can contribute to FE and BE
  • FE and BE run on different processes (and even different hosts)
  • Vscode extensions and Theia plugins run in a separate process (extension host)
  • The illustration can hopefully reflect the RPC communication lines between FE and BE, and FE to extensions host (via BE).
  • There is a one to one relationship between the number of FE and number of plugin hosts.

The above took me sometime to understand and hopefully it would be useful for others,
I hope I got it right :-)

@JonasHelming
Copy link
Contributor Author

Please update the commit message, there are currently typos:

Added detailled description about extenssion types

Added detailed descriptions about extension types

I think there is still some work to be done regarding the overall content of the documentation added to make it clearer and more informative.

Will do when I do the final squash

@JonasHelming
Copy link
Contributor Author

It would be great if the new image for extensions depicts:

  • How @Theia extensions can contribute to FE and BE
  • FE and BE run on different processes (and even different hosts)
  • Vscode extensions and Theia plugins run in a separate process (extension host)
  • The illustration can hopefully reflect the RPC communication lines between FE and BE, and FE to extensions host (via BE).
  • There is a one to one relationship between the number of FE and number of plugin hosts.

The above took me sometime to understand and hopefully it would be useful for others,
I hope I got it right :-)

I agree, all your points should be documented. However, I do not want to overload the diagram here, it is already a little more complex then I wanted it to be. This article is really meant as an overview. I believe your suggestion should go into a section called "architecture" or "extension architecture". Would you mind to paste your suggestion into a separate ticket?

@JonasHelming JonasHelming changed the title Added detailled description about extenssion types Added detailed description about extension types Aug 3, 2021
@alvsan09
Copy link

alvsan09 commented Aug 4, 2021

I agree, all your points should be documented. However, I do not want to overload the diagram here, it is already a little more complex then I wanted it to be. This article is really meant as an overview. I believe your suggestion should go into a section called "architecture" or "extension architecture". Would you mind to paste your suggestion into a separate ticket?

I have created the following issues as per your suggestion: ##186 (comment)

@JonasHelming
Copy link
Contributor Author

@vince-fugnitto : Are you waiting for Marcs review or are we good to go here?

src/docs/extensions.md Outdated Show resolved Hide resolved
src/docs/extensions.md Outdated Show resolved Hide resolved
src/docs/extensions.md Outdated Show resolved Hide resolved
src/docs/extensions.md Show resolved Hide resolved
src/docs/extensions.md Outdated Show resolved Hide resolved
@marcdumais-work marcdumais-work removed their request for review August 11, 2021 15:57
fixed #158

Contributed on behalf of STMicroelectronics

Signed-off-by: Jonas Helming <jhelming@eclipsesource.com>
@JonasHelming
Copy link
Contributor Author

Please update the commit message, there are currently typos:

Added detailled description about extenssion types

Added detailed descriptions about extension types

I think there is still some work to be done regarding the overall content of the documentation added to make it clearer and more informative.

Done!

Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the changes are fine for the moment and provide additional details as to what exists currently 👍 We can always re-visit the documentation to go more in depth (and possibly technical) in future iterations.

@JonasHelming JonasHelming merged commit 0dd8769 into master Aug 25, 2021
@vince-fugnitto vince-fugnitto deleted the GH-158 branch August 25, 2021 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve Extension introduction in documentation
5 participants