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

[issue-139] Improve documentation #166

Merged
merged 1 commit into from
Apr 29, 2024

Conversation

rsearls
Copy link
Collaborator

@rsearls rsearls commented Apr 9, 2024

Please review this major rewrite and restructuring of ./README.md
and provisioners/README.md and provide feedback particularly on
correctness of the information.

Part of #139

@fabiobrz
Copy link
Member

fabiobrz commented Apr 10, 2024

Thank you @rsearls - this is, simply put, great.

It'll take some time to review the changes, but such an improvement will add a huge value, and therefore I pinned this PR to the release milestone.

@rsearls
Copy link
Collaborator Author

rsearls commented Apr 11, 2024

/retest

1 similar comment
@rsearls
Copy link
Collaborator Author

rsearls commented Apr 12, 2024

/retest

Copy link
Member

@fabiobrz fabiobrz 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 @rsearls for this PR.
This is a huge and valuable contribution to the project, so kudos to your commitment.

Most of my comments are just about typos or minor things, like formatting to have a better rendering. A few of them are about the contents.

Feel free to let me know what you think.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
provisioners/README.md Outdated Show resolved Hide resolved
provisioners/README.md Outdated Show resolved Hide resolved
provisioners/README.md Outdated Show resolved Hide resolved
provisioners/README.md Outdated Show resolved Hide resolved
provisioners/README.md Show resolved Hide resolved
@rsearls rsearls force-pushed the x-readme-rewrite branch 2 times, most recently from b3ae4e8 to b47563e Compare April 24, 2024 17:09
@fabiobrz
Copy link
Member

fabiobrz commented Apr 24, 2024

/retest

Should rebase on main, first.

@rsearls
Copy link
Collaborator Author

rsearls commented Apr 25, 2024

/retest

Copy link
Member

@fabiobrz fabiobrz left a comment

Choose a reason for hiding this comment

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

Hi @rsearls - thanks for addressing my comments.
I've dropped a couple more about minor issues, and one about the two tables which are documenting provisioners, those should be merged into one.

Fell free to let me know what you think.

docs/XTF-Configuration-Properties.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
docs/Supported-Product-Components.md Outdated Show resolved Hide resolved
Copy link
Member

@fabiobrz fabiobrz left a comment

Choose a reason for hiding this comment

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

Changes LGTM, I am approving and merging the changes, thanks again @rsearls !

The documentation will be also improved additionally via the changes in #159, where we'll add information about supported versions

@fabiobrz fabiobrz merged commit a924223 into Intersmash:main Apr 29, 2024
1 of 3 checks passed
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.

2 participants