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

Documentation Updates #133

Merged
merged 8 commits into from
Jan 17, 2025

Conversation

jreineckearm
Copy link
Contributor

Updates the README to add some documentation for end users.
This PR focuses on prerequisites and launch configuration settings.

It probably doesn't make sense to extensively document the Memory Browser window which to my understanding is planned to be removed in future when some integration issues with Memory Inspector in the cdt-gdb-adapter are solved.

@jreineckearm
Copy link
Contributor Author

Still a draft. Would like to gather first feedback on the approach before polishing things.

@jreineckearm
Copy link
Contributor Author

Sigh....CI failing. I believe there were some updates regarding Github runners rolled out. In any case, not yet updated dependencies complain. Will look into those tomorrow.

@jreineckearm
Copy link
Contributor Author

CI fix in #134

Copy link
Contributor

@jonahgraham jonahgraham left a comment

Choose a reason for hiding this comment

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

Updates the README to add some documentation for end users.
This PR focuses on prerequisites and launch configuration settings.

These sections look good

It probably doesn't make sense to extensively document the Memory Browser window which to my understanding is planned to be removed in future when some integration issues with Memory Inspector in the cdt-gdb-adapter are solved.

+1 I think the memory browser can be mentioned, but should direct to memory inspector + #110 for people to comment if they have any real use for the one in this extension, but not much more.

I also think everything from https://github.com/jreineckearm/cdt-gdb-vscode/tree/launch-settings-docs?tab=readme-ov-file#building on should be moved out of readme and into a new building.md or similar.

README.md Outdated Show resolved Hide resolved
README.md Outdated
## Launch Settings

TODO
## Prequisites
Copy link
Contributor

Choose a reason for hiding this comment

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

Spelling. I'm not checking for spelling really, I recommend installing https://marketplace.visualstudio.com/items?itemName=streetsidesoftware.code-spell-checker or something similar in vscode to handle spell checking. It wouldn't surprise me if there are misspellings in the launch.json that have been copied into here (like examinging below :-)

Suggested change
## Prequisites
## Prerequisites

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ooops ... thanks! Will run a spell checker on it before it becomes a full PR. And update findings in package.json accordingly.

@jreineckearm jreineckearm force-pushed the launch-settings-docs branch 2 times, most recently from 81756d7 to 5b36f5d Compare January 17, 2025 09:34
@jreineckearm jreineckearm marked this pull request as ready for review January 17, 2025 16:34
jreineckearm and others added 8 commits January 17, 2025 17:37
Signed-off-by: Jens Reinecke <jens.reinecke@arm.com>
Signed-off-by: Jens Reinecke <jens.reinecke@arm.com>
Co-authored-by: Jonah Graham <jonah@kichwacoders.com>
Signed-off-by: Jens Reinecke <jens.reinecke@arm.com>
Signed-off-by: Jens Reinecke <jens.reinecke@arm.com>
Signed-off-by: Jens Reinecke <jens.reinecke@arm.com>
Signed-off-by: Jens Reinecke <jens.reinecke@arm.com>
Signed-off-by: Jens Reinecke <jens.reinecke@arm.com>
Signed-off-by: Jens Reinecke <jens.reinecke@arm.com>
@jreineckearm
Copy link
Contributor Author

Ready now for review after last rebase

Copy link
Contributor

@jonahgraham jonahgraham left a comment

Choose a reason for hiding this comment

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

LGTM

I leave it up to @thegecko to do a review too as I am happy with what is present, but I would appreciate it if someone with experience of publishing to marketplace makes sure everything is here.

Copy link

@thegecko thegecko left a comment

Choose a reason for hiding this comment

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

Great improvements over existing docs 👍

@thegecko thegecko merged commit 7a1414e into eclipse-cdt-cloud:main Jan 17, 2025
5 checks passed
@jreineckearm jreineckearm deleted the launch-settings-docs branch January 18, 2025 16:58
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.

3 participants