Skip to content

Upgrade Readability and Usefulness of Overview.md - #152

Open
cameronhum wants to merge 9 commits into
thoth-tech:mainfrom
cameronhum:overview-upgrade3
Open

cameronhum wants to merge 9 commits into
thoth-tech:mainfrom
cameronhum:overview-upgrade3

Conversation

@cameronhum

Copy link
Copy Markdown
Contributor

Description

Please include a summary of the change and which issue is fixed. Please also include relevant
motivation and context. List any dependencies that are required for this change.

Fixes # (issue)

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as
    expected)
  • Documentation (update or new)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can
reproduce. Please also list any relevant details for your test configuration

Testing Checklist

  • Tested in latest Chrome
  • Tested in latest Safari
  • Tested in latest Firefox

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have requested a review from ... on the Pull Request

@cameronhum
cameronhum force-pushed the overview-upgrade3 branch from 5314147 to b13b509 Compare May 26, 2026 08:44
This reverts commit 5314147. and fixes the overview.md to have the latest.

was using an online markdown editor to live preview. this was then copied into the wrong file so the changes that were in the folder structure overview md are now in overview.md like they were intended

@ayushindapure ayushindapure left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

There are lots of conflicts on this commit including Unformatted Markdown, would you be able to pull and than fix everything

@Arnold-Seb

Arnold-Seb commented Sep 13, 2026

Copy link
Copy Markdown

I checked the branch out, diffed it against thoth-tech/main, and verified the document against the source code.

Good change. The table of contents, the file structure tree and the per file anchors make this navigable instead of a wall of text, and the stale filenames from the old version (executionEnvironment_CodeProcessor.js, loadSplashKit.js, etc.) are now correct. I checked all 106 internal anchors and all four links to other docs, they resolve. The client side initialisation order matches the script order in index.html exactly, and spot checks on server.js, setup.js, SKOservice-worker.js and themes.js all hold.

Two things before I can approve:

  1. .gitignore conflicts with main. I merged main into the branch locally and git stops on it. Main already added _framework/.DS_Store, so merge main in and keep CSharpWasmExpo/wwwroot/ as the only new line. overview.md itself merges clean.
  2. Three paths in the tables do not exist in main:
    • Line 166: the file is at javascript/UI/IDEStartupMain.js, not javascript/startup/. The file structure tree in this PR already has it right, so the table and the tree disagree.
    • Lines 196 and 197: the folder is spelled executionEnviroment, missing the n, so both paths resolve to nothing.
    • Lines 291 and 293: baseTheme.css and stylesheet.css do not exist. index.html loads colours.css, shared.css, index.css and the three component sheets.

Minor, not blocking: the Files Overview subsections are ## so the table of contents nesting does not match the outline, communication.js is missing from the initialisation pipeline, and the file structure tree leaves out Documentation/.

Fix those two and I will approve.

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