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

feat(doc): add overview content to bao-classic config documentation #66

Merged
merged 1 commit into from
Dec 7, 2023

Conversation

Diogo21Costa
Copy link
Member

PR Description

In this pull request, I'm adding content to the "Overview" section of the getting started documentation for the Bao hypervisor.

This update focuses solely on the "Overview" section, providing a detailed description of the configuration file and its different available fields.

Do you have any suggestions or feedback concerning the content provided in the "Overview" section?

Copy link
Member

@danielRep danielRep left a comment

Choose a reason for hiding this comment

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

IMHO the overview section sufficiently explains at a high-level the need and use of the config file. Just added some format typos, but don't forget to check trailing whitespaces and probably lines will surpass the characters limit.

source/bao_hyp/config.rst Outdated Show resolved Hide resolved
source/bao_hyp/config.rst Outdated Show resolved Hide resolved
source/bao_hyp/config.rst Outdated Show resolved Hide resolved
source/bao_hyp/config.rst Outdated Show resolved Hide resolved
source/bao_hyp/config.rst Outdated Show resolved Hide resolved
source/bao_hyp/config.rst Outdated Show resolved Hide resolved
source/bao_hyp/config.rst Outdated Show resolved Hide resolved
@Diogo21Costa Diogo21Costa force-pushed the feat/bao-classic_config_overview branch from 3311afc to d120e6f Compare July 27, 2023 15:07
source/bao_hyp/config.rst Outdated Show resolved Hide resolved
source/bao_hyp/config.rst Outdated Show resolved Hide resolved
source/bao_hyp/config.rst Outdated Show resolved Hide resolved
source/bao_hyp/config.rst Outdated Show resolved Hide resolved
source/bao_hyp/config.rst Outdated Show resolved Hide resolved
source/bao_hyp/config.rst Outdated Show resolved Hide resolved
source/bao_hyp/config.rst Outdated Show resolved Hide resolved
source/bao_hyp/config.rst Outdated Show resolved Hide resolved
source/bao_hyp/config.rst Outdated Show resolved Hide resolved
source/bao_hyp/config.rst Outdated Show resolved Hide resolved
@Diogo21Costa Diogo21Costa force-pushed the feat/bao-classic_config_overview branch 3 times, most recently from 2514d46 to 69e5cd1 Compare July 27, 2023 17:12
source/bao_hyp/config.rst Outdated Show resolved Hide resolved
source/bao_hyp/config.rst Outdated Show resolved Hide resolved
@danielRep
Copy link
Member

Don't forget to squash all the commits into only one meaningful commit @Diogo21Costa to maintain the history clean.

@Diogo21Costa Diogo21Costa force-pushed the feat/bao-classic_config_overview branch 2 times, most recently from bb9016a to 326d3bb Compare August 9, 2023 15:28
source/bao_hyp/config.rst Outdated Show resolved Hide resolved
source/bao_hyp/config.rst Outdated Show resolved Hide resolved
@Diogo21Costa Diogo21Costa force-pushed the feat/bao-classic_config_overview branch from f4afd64 to a38a4a8 Compare September 12, 2023 14:04
@Diogo21Costa
Copy link
Member Author

#66 (comment)

@danielRep @josecm I've improved the guest load description by including a description of the VM_IMAGE macro and a pointer to the 'Guest Image' section, where later it will be described how the guest image is configured. Can you please provide feedback on this change?

source/bao_hyp/config.rst Outdated Show resolved Hide resolved
source/bao_hyp/config.rst Outdated Show resolved Hide resolved
source/bao_hyp/config.rst Outdated Show resolved Hide resolved
source/bao_hyp/config.rst Outdated Show resolved Hide resolved
source/bao_hyp/config.rst Outdated Show resolved Hide resolved
source/bao_hyp/config.rst Outdated Show resolved Hide resolved
source/bao_hyp/config.rst Outdated Show resolved Hide resolved
source/bao_hyp/config.rst Outdated Show resolved Hide resolved
Copy link
Member

@danielRep danielRep left a comment

Choose a reason for hiding this comment

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

Small format tweaks @Diogo21Costa . After these small changes, is fine by me.

source/bao_hyp/config.rst Outdated Show resolved Hide resolved
source/bao_hyp/config.rst Outdated Show resolved Hide resolved
source/bao_hyp/config.rst Outdated Show resolved Hide resolved
source/bao_hyp/config.rst Outdated Show resolved Hide resolved
source/bao_hyp/config.rst Outdated Show resolved Hide resolved
@Diogo21Costa
Copy link
Member Author

Small format tweaks @Diogo21Costa . After these small changes, is fine by me.

@danielRep Fully agree with all the points, thank you for the review! Applied all the changes in 133b776

Currently, the gitlint checker is failing due to unsigned commits, in specific the following ones:

77e63cf - fix(doc): improve documentation readability
7613bf8) - fix(doc): improve documentation readability
d91a99d - fix(doc): improve documentation readability
8678de3 - fix(doc): improve documentation readability
a02d816 - fix(doc): improve documentation readability
94a87a5 - fix(doc): improve documentation readability

To solve the issue and to maintain the history clean, may I squash all the "fix(doc)" commits into a single one?

source/bao_hyp/config.rst Outdated Show resolved Hide resolved
danielRep
danielRep previously approved these changes Sep 27, 2023
Copy link
Member

@danielRep danielRep left a comment

Choose a reason for hiding this comment

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

LGTM. Don't forget to review the git history.

@danielRep
Copy link
Member

Small format tweaks @Diogo21Costa . After these small changes, is fine by me.

@danielRep Fully agree with all the points, thank you for the review! Applied all the changes in 133b776

Currently, the gitlint checker is failing due to unsigned commits, in specific the following ones:

77e63cf - fix(doc): improve documentation readability 7613bf8) - fix(doc): improve documentation readability d91a99d - fix(doc): improve documentation readability 8678de3 - fix(doc): improve documentation readability a02d816 - fix(doc): improve documentation readability 94a87a5 - fix(doc): improve documentation readability

To solve the issue and to maintain the history clean, may I squash all the "fix(doc)" commits into a single one?

Yes, all those commits should be squashed.

Copy link
Member

@josecm josecm left a comment

Choose a reason for hiding this comment

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

I think this still needs a small iteration.

Please, also clean the commit history.

source/bao_hyp/config.rst Outdated Show resolved Hide resolved
source/bao_hyp/config.rst Outdated Show resolved Hide resolved
source/bao_hyp/config.rst Outdated Show resolved Hide resolved
source/bao_hyp/config.rst Outdated Show resolved Hide resolved
source/bao_hyp/config.rst Outdated Show resolved Hide resolved
source/bao_hyp/config.rst Outdated Show resolved Hide resolved
source/bao_hyp/config.rst Outdated Show resolved Hide resolved
source/bao_hyp/config.rst Outdated Show resolved Hide resolved
source/bao_hyp/config.rst Outdated Show resolved Hide resolved
source/bao_hyp/config.rst Outdated Show resolved Hide resolved
@josecm
Copy link
Member

josecm commented Dec 5, 2023

@Diogo21Costa you should also refactor this to use the new column limit of 100.

@danielRep
Copy link
Member

danielRep commented Dec 6, 2023

Fix the checker issues when you have the time @Diogo21Costa . You need to rebase the branches with main to avoid the format-check triggering and rewriting the git history too.

@Diogo21Costa Diogo21Costa force-pushed the feat/bao-classic_config_overview branch from c7f6a81 to 89299c7 Compare December 6, 2023 15:33
josecm
josecm previously approved these changes Dec 6, 2023
@josecm josecm requested a review from danielRep December 6, 2023 15:40
@josecm
Copy link
Member

josecm commented Dec 6, 2023

@Diogo21Costa please fix the commit history and messages, as well as the formatting errors.

@Diogo21Costa Diogo21Costa force-pushed the feat/bao-classic_config_overview branch from 89299c7 to 2dec63a Compare December 7, 2023 11:11
@danielRep danielRep force-pushed the wip/bao-classic_config branch 2 times, most recently from 40f6ede to 1ccdf58 Compare December 7, 2023 11:44
Signed-off-by: Diogo21Costa <diogoandreveigacosta@gmail.com>
@Diogo21Costa Diogo21Costa force-pushed the feat/bao-classic_config_overview branch from 2dec63a to 6619845 Compare December 7, 2023 12:48
@Diogo21Costa
Copy link
Member Author

@josecm @danielRep Fixed the commit history and the formatting errors, thanks for the review!

@danielRep danielRep merged commit 8cfce09 into wip/bao-classic_config Dec 7, 2023
4 checks passed
@danielRep danielRep deleted the feat/bao-classic_config_overview branch December 7, 2023 14:22
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