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.

Sign up for GitHub

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

Jump to bottom

Add man page #80

Merged
merged 4 commits into from
Jan 24, 2023
Merged

Add man page #80

merged 4 commits into from
Jan 24, 2023

Conversation

tylerfanelli
Copy link
Member

No description provided.

@tylerfanelli tylerfanelli requested review from slp and crobinso November 17, 2022 22:41
@slp
Copy link
Contributor

slp commented Nov 18, 2022

Hm... I wonder if committing a pre-formatted man page is the best approach here. I kind of prefer what we did on krunvm, committing them as text files and generating the man pages using asciidoctor: containers/krunvm@8244dd6

@tylerfanelli tylerfanelli marked this pull request as draft November 18, 2022 19:53
@tylerfanelli
Copy link
Member Author

tylerfanelli commented Nov 18, 2022 •
edited
Loading

@slp Yea, I like your suggestion better. Will request re-review once changes are made.

@tylerfanelli tylerfanelli force-pushed the manpage branch 13 times, most recently from 0fbe8e8 to 8da6792 Compare December 7, 2022 02:09
@tylerfanelli tylerfanelli marked this pull request as ready for review December 7, 2022 02:10
@tylerfanelli
Copy link
Member Author

@slp @crobinso When you get the chance, can you review?

@crobinso
Copy link
Contributor

Is the plan to kill cargo-readme usage? I'm in favor of that. There should be only one source of truth for long form command descriptions, man page makes sense. Could happen as a follow on commit though.

I see the COMMAND OVERVIEW section is just a copy of
README.md content. Content is redundant, and not sure I've seen a section like that in a man page before. I think there should just be a COMMANDS section, like in man systemctl

If you rename the file to sevctl.1.adoc I think we get nicer github formatting

@tylerfanelli tylerfanelli force-pushed the manpage branch 4 times, most recently from 18b6da2 to a5da18f Compare January 19, 2023 04:39
@tylerfanelli
ci: Add asciidoctor as a build dependency
29ddeb7 
asciidoctor will now be a build dependency for sevctl. With this, a
sevctl man page will now be able to be built from asciidoctor.

Signed-off-by: Tyler Fanelli <tfanelli@redhat.com>
@tylerfanelli tylerfanelli force-pushed the manpage branch 2 times, most recently from 1ad6640 to eb29bff Compare January 19, 2023 04:44
@tylerfanelli
docs: Add a generable man page
0800669 
build.rs allows asciidoctor to generate a manpage for sevctl every time
this package is built.

Signed-off-by: Tyler Fanelli <tfanelli@redhat.com>
@tylerfanelli
.github: Remove cargo readme CI test
9a38002 
With the introduction of a man page, all releveant command usage
information can be stored there, instead of crowding src/main.rs with
all information in the README.

Signed-off-by: Tyler Fanelli <tfanelli@redhat.com>
@tylerfanelli
src/main: Remove README portion of main.rs
e78d8c1 
Signed-off-by: Tyler Fanelli <tfanelli@redhat.com>
@tylerfanelli
Copy link
Member Author

Is the plan to kill cargo-readme usage? I'm in favor of that. There should be only one source of truth for long form command descriptions, man page makes sense. Could happen as a follow on commit though.

Yea, I agree. I added a commit removing the usage of cargo readme.

I see the COMMAND OVERVIEW section is just a copy of README.md content. Content is redundant, and not sure I've seen a section like that in a man page before. I think there should just be a COMMANDS section, like in man systemctl

I see what you mean, section removed.

If you rename the file to sevctl.1.adoc I think we get nicer github formatting

Done.

@tylerfanelli
Copy link
Member Author

@crobinso When you have the chance, can you re-review?

crobinso
crobinso approved these changes Jan 24, 2023
View reviewed changes
Copy link
Contributor

@crobinso crobinso left a comment

Choose a reason for hiding this comment

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

LGTM

@tylerfanelli tylerfanelli merged commit e37c4d6 into virtee:main Jan 24, 2023
@tylerfanelli tylerfanelli deleted the manpage branch July 6, 2023 23:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@crobinso crobinso crobinso approved these changes

@slp slp Awaiting requested review from slp

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tylerfanelli @slp @crobinso