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

Website for Theia Trace Extension documentation #896

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Rodrigoplp-work
Copy link
Contributor

@Rodrigoplp-work Rodrigoplp-work commented Dec 20, 2022

Adds a website in the "doc" folder to hold the documentation for Theia Trace Extension.

Website built with Svelte Kit.

This PR replaces #776, which accomplishes the same goal but uses GatsbyJS. The proposal to replace Gatsby with Svelte aims at reducing the code base and adopting more modern solutions for the problem of rendering static documentation sites.

Signed-off-by: Rodrigo Pinto rodrigo.pinto@calian.ca

@frallax
Copy link
Contributor

frallax commented Dec 22, 2022

Great that someone got back on this one!
I have no issues in using another framework than Gatsby, just that we get some documentation in place.

However, keep in mind that #776 was developed with the following requirements:

  • the static webpage should be able to to render mermaid diagrams
  • the static webpage should be able to offer code highlight
  • the web-doc folder should contain only the "static webpage generation framework" and a few md files (for the main page)
    • i.e. the rest of the pages should be generated from md files in the folders under theia-trace-extension/doc, for example the "adr" folder
  • it is preferred that the md files used to generate the pages (e.g. the ones under "adr") do not use annotations/front matter
  • there should be a minimal sidebar to browse the pages. The content of the sidebar should be auto generated based on which md files are used to generate the pages (e.g. the ones under "adr")
  • the static webpage should be used to browse both user oriented documentation (e.g. intro, tutorials, etc.) and developer documentation (e.g. adrs)
  • it is preferred that the framework used to generate the webpage is compatible with the same node version used for the theia-trace-extension

The reason for many of the requirements above: we wanted a "static webpage generation framework" independent solution.
Considering the speed with which new frameworks born and die in the js community, we were envisioning the possibility to "just" change the framework in the web-doc folder and get a new site (based on the new framework) without having to move/reorganize md files used for the webpage generation.

I hope the explanation above clarifies the proof of concept presented in #776.

So, do we fully want to replace the proof-of-concept presented in #776? In that case I'd be extremely interested to check if your proposal can address the requirements above.
Otherwise we can just relax some of the requirements above (with some consequences), but I never got so much momentum from the community to discuss this in #776.

Some other side notes:

  • a feedback I got in WIP: propose a framework for documentation #776, was that the PR was "too big" mainly due to the package-lock file. It is great to see that your proposal is "leaner" (also if now it does not contain all features listed above).
  • I will try to investigate why WIP: propose a framework for documentation #776 package-lock file got so big (can it be that you use yarn instead of nmp? or is it due to the extra plugins I added to support the features above?), but if it shows that svelte generates less deps to get the same features, I think just that would be a great find!
  • I tried your updated PR, with node 16.15.1 (nvm use lts/gallium) and I still see the same issues. I run the following commands:
    • yarn
    • yarn dev --open'
    • I open the browser at http://127.0.0.1:5173/
    • I see a 500 internal error and the shell shows:
Failed to load url /src/lib/components/Header.svelte (resolved id: /src/lib/components/Header.svelte). Does the file exist?
Failed to load url /src/lib/components/Sidebar.svelte (resolved id: /src/lib/components/Sidebar.svelte). Does the file exist?

@Rodrigoplp-work Rodrigoplp-work force-pushed the webdoc branch 2 times, most recently from f19d37d to 4829c31 Compare December 22, 2022 16:49
@Rodrigoplp-work
Copy link
Contributor Author

Thank you for considering this, Francesco.

The second commit implements all items from your list of requirements, including the Mermaid diagrams. The images inside the ADR's are not showing up, but I can fix that once I have the chance.

The minimum NodeJS version required for this PR is the same as for theia-trace-extension, which is the same as for Theia.

This PR is an attempt to avoid adding an old, larger and less efficient framework to the project.

I hope it is not dismissed as if it was using a framework that is "born and dies" overnight, because Svelte has been between the most loved frameworks for years now. (See https://survey.stackoverflow.co/2022/#most-loved-dreaded-and-wanted-webframe-love-dread).

The JS community evolves and we should take advantage of its progress.

Copy link
Contributor

@marco-miller marco-miller left a comment

Choose a reason for hiding this comment

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

This PR would require being rebased on current latest master, before I at least can review it; thanks. It is currently many commits behind already. Time flies indeed :)

@Rodrigoplp-work
Copy link
Contributor Author

Rebased.

@marco-miller
Copy link
Contributor

Rebased.

Thanks! -Will get back to it early next week as soon as possible.

Copy link
Contributor

@marco-miller marco-miller left a comment

Choose a reason for hiding this comment

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

Thanks for this work. My initial comments before reviewing further, mainly README ones. And here are my general comments:

  1. I'd split the styling part out of the ADR commit, so into another commit that follows. This is so that we may review the moves separately from the in-file changes.
  2. Moving the ADRs would also require the corresponding changes to repo root's .vscode/launch.json and README.md.

doc/web-doc/README.md Outdated Show resolved Hide resolved
doc/web-doc/README.md Outdated Show resolved Hide resolved
doc/web-doc/README.md Outdated Show resolved Hide resolved
doc/web-doc/README.md Show resolved Hide resolved
doc/web-doc/README.md Outdated Show resolved Hide resolved
doc/web-doc/README.md Outdated Show resolved Hide resolved
doc/web-doc/README.md Outdated Show resolved Hide resolved
doc/web-doc/README.md Outdated Show resolved Hide resolved
doc/web-doc/README.md Outdated Show resolved Hide resolved
doc/web-doc/README.md Outdated Show resolved Hide resolved
@bhufmann
Copy link
Collaborator

@Rodrigoplp-work Does this require to run a web-server? Or can it be statically linked from, for example, https://www.eclipse.org/cdt-cloud/. Please let me know.

@marco-miller
Copy link
Contributor

marco-miller commented Jan 16, 2023

Addendum:
3. the About lends a repeating About Theia Trace Extension;
4. should we instead show GitHub's own rendered pages for the ADRs (and other potential documents), rather than rendering them again locally -and differently- within this site?
5. Which other existing documents should be added now to this inital site, if any? (If some, should we also include them now?)

@Rodrigoplp-work Rodrigoplp-work force-pushed the webdoc branch 2 times, most recently from c982ac8 to af15ae5 Compare January 18, 2023 14:57
@Rodrigoplp-work
Copy link
Contributor Author

@Rodrigoplp-work Does this require to run a web-server? Or can it be statically linked from, for example, https://www.eclipse.org/cdt-cloud/. Please let me know.

Yes, it requires a web-server, which is started with yarn dev typed at /doc/web-doc/ folder. It cannot be linked from cdt-cloud because the webpages are not available online, only locally.

This is the requirement proposed by Francesco in his initial PR (#776). As he stated:

"The same approach could be used to generate user oriented
documentation, but it is outside the scope of this commit."

@Rodrigoplp-work
Copy link
Contributor Author

Addendum: 3. the About lends a repeating About Theia Trace Extension; 4. should we instead show GitHub's own rendered pages for the ADRs (and other potential documents), rather than rendering them again locally -and differently- within this site? 5. Which other existing documents should be added now to this inital site, if any? (If some, should we also include them now?)

  1. Like WIP: propose a framework for documentation #776, the purpose of this PR is to prove the effectivity of the framework against our requirements. We should work on the content in future PRs;
  2. Becoming independent of third parties, like GitHub, is one of the greatest advantages of rendering our own pages. It shields us from having to accept future changes they may execute, when those don't agree with what we want;
  3. None, as the present content already illustrates the framework can match all requirements (apart from the ADR images, but we already have examples for that). Just like WIP: propose a framework for documentation #776, the purpose of this PR is to prove the framework is appropriate. Further content should be added in separate PRs.

@Rodrigoplp-work
Copy link
Contributor Author

Thanks for this work. My initial comments before reviewing further, mainly README ones. And here are my general comments:

  1. I'd split the styling part out of the ADR commit, so into another commit that follows. This is so that we may review the moves separately from the in-file changes.
  2. Moving the ADRs would also require the corresponding changes to repo root's .vscode/launch.json and README.md.
  1. Done;
  2. Fixed.

@bhufmann
Copy link
Collaborator

bhufmann commented Jan 18, 2023

@Rodrigoplp-work Does this require to run a web-server? Or can it be statically linked from, for example, https://www.eclipse.org/cdt-cloud/. Please let me know.

Yes, it requires a web-server, which is started with yarn dev typed at /doc/web-doc/ folder. It cannot be linked from cdt-cloud because the webpages are not available online, only locally.

This is the requirement proposed by Francesco in his initial PR (#776). As he stated:

"The same approach could be used to generate user oriented documentation, but it is outside the scope of this commit."

The web-server would have to run at Eclipse foundation premises. I'm not clear if and how we can get such setup.

Each Eclipse project has a location that is served by the central web server https://www.eclipse.org/ and each project has a dedicated site https://www.eclipse.org/project. I think the dedicated site has to be populated by a static web content.

For example:

The theia-trace-extension is part of the cdt-cloud project and hence it's referenced on the cdt-cloud project page. The source of the webpage is under the repo https://github.com/eclipse-cdt-cloud/website and is then deployed to the publish repo https://github.com/eclipse-cdt-cloud/website-publish which serves https://www.eclipse.org/cdt-cloud.

The https://www.eclipse.org/tracecompass page is for Eclipse Trace Compass and is served by repo https://github.com/eclipse-tracecompass/tracecompass-website.

Any suggestions on how to continue with this PR and take advantage of Svelte considering the information above? Can it be deployed to GitHub pages?

@marco-miller
Copy link
Contributor

  1. Like WIP: propose a framework for documentation #776, the purpose of this PR is to prove the effectivity of the framework against our requirements. We should work on the content in future PRs;
    Marco: I agree with you that further content should be other PRs. Now, I think that the minimal content this PR brings (e.g., the About) should be minimally provisioned with real content. -Unless we remove the section for this PR and seek another PR for it indeed.
  1. Becoming independent of third parties, like GitHub, is one of the greatest advantages of rendering our own pages. It shields us from having to accept future changes they may execute, when those don't agree with what we want;
    Marco: the reviewed ADRs or other documents that we merge in GH should be deemed authoritative. Thus this question I had about the risk of redoing rendering logic that's already done by GH or the like. It is of course ok to render our own pages, now I'd be for not duplicating the rendering of already rendered pages of ours.

Copy link
Contributor

@marco-miller marco-miller left a comment

Choose a reason for hiding this comment

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

Thanks again @Rodrigoplp-work and @frallax for this whole effort in improving our online doc.

doc/web-doc/README.md Outdated Show resolved Hide resolved
doc/web-doc/README.md Outdated Show resolved Hide resolved
@Rodrigoplp-work
Copy link
Contributor Author

@Rodrigoplp-work Does this require to run a web-server? Or can it be statically linked from, for example, https://www.eclipse.org/cdt-cloud/. Please let me know.

Yes, it requires a web-server, which is started with yarn dev typed at /doc/web-doc/ folder. It cannot be linked from cdt-cloud because the webpages are not available online, only locally.
This is the requirement proposed by Francesco in his initial PR (#776). As he stated:
"The same approach could be used to generate user oriented documentation, but it is outside the scope of this commit."

The web-server would have to run at Eclipse foundation premises. I'm not clear if and how we can get such setup.

Each Eclipse project has a location that is served by the central web server https://www.eclipse.org/ and each project has a dedicated site https://www.eclipse.org/project. I think the dedicated site has to be populated by a static web content.

For example:

The theia-trace-extension is part of the cdt-cloud project and hence it's referenced on the cdt-cloud project page. The source of the webpage is under the repo https://github.com/eclipse-cdt-cloud/website and is then deployed to the publish repo https://github.com/eclipse-cdt-cloud/website-publish which serves https://www.eclipse.org/cdt-cloud.

The https://www.eclipse.org/tracecompass page is for Eclipse Trace Compass and is served by repo https://github.com/eclipse-tracecompass/tracecompass-website.

Any suggestions on how to continue with this PR and take advantage of Svelte considering the information above? Can it be deployed to GitHub pages?

Sure.

I can see that https://github.com/eclipse-cdt-cloud/website is built on Hugo, which is a static site generator.

I don't know what eclipse.org is built on. It could also be Hugo, as it seems to use the eclipse-cdt-cloud/website repo to populate the eclipse.org/cdt-cloud url.

Anyway, SvelteKit can generate static websites too. By adding an adapter (https://kit.svelte.dev/docs/adapters#supported-environments-static-sites) it creates a folder in https://github.com/eclipse-cdt-cloud/theia-trace-extension/tree/master/doc/web-doc containing the whole project as a static website.

Then, eclipse.org could create a page from that folder the same way it does from the https://github.com/eclipse-tracecompass/tracecompass-website repo.

@frallax
Copy link
Contributor

frallax commented Jan 19, 2023

Thank you for considering this, Francesco.

The second commit implements all items from your list of requirements, including the Mermaid diagrams. The images inside the ADR's are not showing up, but I can fix that once I have the chance.

The minimum NodeJS version required for this PR is the same as for theia-trace-extension, which is the same as for Theia.

This PR is an attempt to avoid adding an old, larger and less efficient framework to the project.

I hope it is not dismissed as if it was using a framework that is "born and dies" overnight, because Svelte has been between the most loved frameworks for years now. (See https://survey.stackoverflow.co/2022/#most-loved-dreaded-and-wanted-webframe-love-dread).

The JS community evolves and we should take advantage of its progress.

I told you that this PR would have had more success than mine ;)

As discussed in a separate forum (but forgot to add here), it looks like the current patchset fulfill all requirements apart from one:

  • the web-doc folder should contain only the "static webpage generation framework" and a few md files (for the main page)
    • i.e. the rest of the pages should be generated from md files in the folders under theia-trace-extension/doc, for example the "adr" folder

The rationale for the requirement above was that we were expecting to have a folder structure similar to the following:

doc/
-- adr/ [our adr friends]
-- user-doc/ [containing only md files with content similar to the trace-compass user guide]
-- developer-doc/ [containing only md files with content similar to the trace-compass developer guide]
-- ... <other topics doc> ...
-- web-doc/ [containing exclusively the framework to render md and generate the documentation]

what you propose in this PR is to move all the documentation (md files) into the doc/web-doc folder.
This could be ok, but not optimal for people who prefer to read the documentation by just cloning the repo and browsing the md files, or for the ones who want to read documentation through github.

Not sure however if this is a "hard" or "soft" requirement, I leave the last word to the project owners :)
Great work btw!

@Rodrigoplp-work Rodrigoplp-work force-pushed the webdoc branch 3 times, most recently from efd503f to 6a6e43c Compare January 19, 2023 22:10
Copy link
Contributor

@marco-miller marco-miller left a comment

Choose a reason for hiding this comment

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

  1. PR can be rebased again at this later stage.
  2. Commit mentions moving ADRs back to their original path, but I'd rather not move them in a preceeding commit in the first place; doing so becomes a known no-op off this PR. IIUC that is.
  3. Wondering if it is necessary to copy the ADRs to a folder inside the project, or can't we instead have that script refer to them from upwards. But maybe we can't.
  4. If we copy still, what if an ADR file changed since previously copied, will it be copied again to update the local doc version of it? (I think it should.) -Asking because commit says only copies folders and files that are missing.
  5. Reiterating a previous comment of mine, about The app automatically generates pages for the ADRs, I'd rather instead reuse (point to) the existing GitHub rendering with Mermaid and all. As opposed to having to maintain a parallel way of rendering such markdowns again, our (specific) way.
  6. Other known sites use Hugo, so maybe we'd need to as well, per this PR and next steps.

doc/web-doc/doc-loader.js Show resolved Hide resolved
doc/web-doc/src/routes/about/+page.svelte Outdated Show resolved Hide resolved
.vscode/launch.json Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
doc/web-doc/.gitignore Outdated Show resolved Hide resolved
@Rodrigoplp-work Rodrigoplp-work force-pushed the webdoc branch 6 times, most recently from 9788eab to 701ddfe Compare January 25, 2023 20:30
@Rodrigoplp-work
Copy link
Contributor Author

Rodrigoplp-work commented Jan 25, 2023

  1. PR can be rebased again at this later stage.
  2. Commit mentions moving ADRs back to their original path, but I'd rather not move them in a preceeding commit in the first place; doing so becomes a known no-op off this PR. IIUC that is.
  3. Wondering if it is necessary to copy the ADRs to a folder inside the project, or can't we instead have that script refer to them from upwards. But maybe we can't.
  4. If we copy still, what if an ADR file changed since previously copied, will it be copied again to update the local doc version of it? (I think it should.) -Asking because commit says only copies folders and files that are missing.
  5. Reiterating a previous comment of mine, about The app automatically generates pages for the ADRs, I'd rather instead reuse (point to) the existing GitHub rendering with Mermaid and all. As opposed to having to maintain a parallel way of rendering such markdowns again, our (specific) way.
  6. Other known sites use Hugo, so maybe we'd need to as well, per this PR and next steps.
  1. Rebased;

  2. Indeed. Commits reset and git history fixed;

  3. Yes, it is necessary to copy the markdown files inside the project. The library that builds the HTML pages can't access files outside the project. This is the same with Gatsby, as configured in the PR put forth by Francesco;

  4. Good point! I changed the script to check for changes in the ADRs that were already copied. Now they will be updated in case of any discrepancies;

  5. That takes away from us the chance of becoming platform agnostic. Markdown with code highlighting and Mermaid diagrams works fine in GitHub, but nothing stops Microsoft from putting that behind a $ 199 yearly subscription or some similar paywall one day.
    It also diminishes the advantages of building our own documentation. We can add a floating div with the headers of an ADR to the right side of the text to facilitate navigation, for example, which we cannot do in GitHub.
    If all we do is link to GitHub then we don't need Svelte, Gatsby, Hugo or any other framework. A simple readme with links will suffice.
    I added a "read this in GitHub" link at the top of each ADR page, so we can have both ways. Does that help address your concerns?

  6. Hugo is a templating language, which is far more limiting than Svelte or Gatsby. Its advantage is offering a simple and fast way to render static sites, but as this and Francesco's PR prove, we achieved that too. There is no more investment necessary to reach the same goal. And using Svelte provides, among others, the advantages pointed in WIP: propose a framework for documentation #776 (comment), Website for Theia Trace Extension documentation #896 and Website for Theia Trace Extension documentation #896.

@Rodrigoplp-work Rodrigoplp-work marked this pull request as ready for review January 25, 2023 20:40
Copy link
Contributor

@marco-miller marco-miller left a comment

Choose a reason for hiding this comment

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

  • LGTM as a base framework now, thanks @Rodrigoplp-work for having pursued this effort.
  • Left to fix would be replacing parentheses with angle brackets in the base commit message Signed-off-by (as well as PR description).
  • I may then Approve and let another review unfold.
  • Our next steps beyond this PR, among others, could be to
  1. check for static generation or how to fit into our Eclipse site, and
  2. look at ways to adjust the page design, based on that integration (style, format, UX &co.).

Adds a website in the "doc" folder to hold the documentation for Theia
Trace Extension.

Website built with [Svelte Kit](https://kit.svelte.dev).

This PR replaces eclipse-cdt-cloud#776, which accomplishes the same goal but uses
GatsbyJS. The proposal to replace Gatsby with Svelte aims at reducing
the code base and adopting more modern solutions for the problem of
rendering static documentation sites.

Signed-off-by: Rodrigo Pinto <rodrigo.pinto@calian.ca>
The script copies ADRs from the `doc/adr` folder into the web-doc
project, so that HTML pages can be generated for them.

The script runs automatically when the user starts the project with
`yarn dev`.

Before copying the files the script compares folders and only copies
folders and files that are missing, or that were modified.

The script runs synchronously, so that the project only starts after the
ADRs have been copied.

Signed-off-by: Rodrigo Pinto <rodrigo.pinto@calian.ca>
Adds pages for ADRs with code styling and mermaid diagrams.

About page has some minimal information about the project that can be
expanded in the future.

Signed-off-by: Rodrigo Pinto <rodrigo.pinto@calian.ca>
Copy link
Contributor

@marco-miller marco-miller left a comment

Choose a reason for hiding this comment

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

Thanks!

@bhufmann
Copy link
Collaborator

bhufmann commented Feb 3, 2023

@Rodrigoplp-work Does this require to run a web-server? Or can it be statically linked from, for example, https://www.eclipse.org/cdt-cloud/. Please let me know.

Yes, it requires a web-server, which is started with yarn dev typed at /doc/web-doc/ folder. It cannot be linked from cdt-cloud because the webpages are not available online, only locally.

This is the requirement proposed by Francesco in his initial PR (#776). As he stated:

"The same approach could be used to generate user oriented documentation, but it is outside the scope of this commit."

The question about deployment still has not been answered, unless I have missed it. Running an additional web-server is not an option as far as I can see. We need to work with what is provided by the Eclipse foundation. Any additional web-server would need to run somewhere and some organization would have to provide the resources for it.

If we can create a static web page from this proposal using Svelte then we can go forward with this proposal. Any thoughts?

@MatthewKhouzam
Copy link
Contributor

Is the doc going to be deployed in FOSS? Asking

Copy link
Contributor

@marco-miller marco-miller left a comment

Choose a reason for hiding this comment

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

Un-approving only to make the open topics visible.

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.

5 participants