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

Fix man page heading formatting #1041

Merged
merged 1 commit into from Jan 16, 2020
Merged

Conversation

saschagrunert
Copy link
Member

@saschagrunert saschagrunert commented Jan 14, 2020

What type of PR is this?

  • bug
  • cleanup
  • documentation
  • feature

What this PR does / why we need it:

Fixing the man page header to display the section correctly

Which issue(s) this PR fixes:

Fixes #1040

Special notes for your reviewer:

None

Testing

Adapted the unit test results

Release Notes

- Fixed invalid man page header generation
- Removed the author from generated man pages

@saschagrunert saschagrunert requested a review from a team as a code owner January 14, 2020 14:22
@saschagrunert saschagrunert requested review from rliebz and coilysiren and removed request for a team January 14, 2020 14:22
@zhsj
Copy link
Contributor

zhsj commented Jan 14, 2020

图片

Is the author section rendered as expected? The first author is title, and the email and other are body.

@saschagrunert
Copy link
Member Author

图片

Is the author section rendered as expected? The first author is title, and the email and other are body.

That's a good question, I think we should put the author to the bottom of the man page, right?

@asahasrabuddhe
Copy link
Member

图片
Is the author section rendered as expected? The first author is title, and the email and other are body.

That's a good question, I think we should put the author to the bottom of the man page, right?

I think you should check this out https://linux.die.net/man/7/man-pages

Screenshot 2020-01-14 at 8 16 03 PM

Although this page discourages adding an Authors section, it is worth noting that the position of the Authors section is second to last.

coilysiren
coilysiren previously approved these changes Jan 14, 2020
Copy link
Member

@coilysiren coilysiren left a comment

Choose a reason for hiding this comment

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

There are some quirks of man page formatting that I haven't taken to the time look into, but this PR LGTM

Closes urfave#1040

Signed-off-by: Sascha Grunert <sgrunert@suse.com>
@saschagrunert
Copy link
Member Author

Okay, I removed the author completely I think that might be something we can agree on.

@codecov
Copy link

codecov bot commented Jan 15, 2020

Codecov Report

Merging #1041 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #1041   +/-   ##
======================================
  Coverage    72.6%   72.6%           
======================================
  Files          33      33           
  Lines        2482    2482           
======================================
  Hits         1802    1802           
  Misses        569     569           
  Partials      111     111

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ee9b3fc...5b3508d. Read the comment docs.

Copy link
Member

@asahasrabuddhe asahasrabuddhe left a comment

Choose a reason for hiding this comment

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

LGTM 👍

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.

v2 bug: generated manpage has error in metadata
4 participants