Skip to content

Fixes the man pages creation#4779

Merged
ThomasWaldmann merged 2 commits intoborgbackup:masterfrom
Gu1nness:issue-4752-build-man-is-broken
Oct 19, 2019
Merged

Fixes the man pages creation#4779
ThomasWaldmann merged 2 commits intoborgbackup:masterfrom
Gu1nness:issue-4752-build-man-is-broken

Conversation

@Gu1nness
Copy link
Copy Markdown
Contributor

@Gu1nness Gu1nness commented Oct 9, 2019

The issue #4471 solution
created many commands for borgfs which shouldn't exist in addition to
creating issue #4752.

Fixes #4752 .

The issue borgbackup#4471 solution
created many commands for borgfs which shouldn't exist in addition to
creating issue borgbackup#4752.

Fixes borgbackup#4752 .
@Gu1nness Gu1nness force-pushed the issue-4752-build-man-is-broken branch from 5df4f43 to dd92695 Compare October 9, 2019 13:15
@codecov-io
Copy link
Copy Markdown

codecov-io commented Oct 9, 2019

Codecov Report

Merging #4779 into master will increase coverage by 0.12%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4779      +/-   ##
==========================================
+ Coverage   83.84%   83.97%   +0.12%     
==========================================
  Files          37       37              
  Lines        9762     9763       +1     
  Branches     1622     1621       -1     
==========================================
+ Hits         8185     8198      +13     
+ Misses       1103     1095       -8     
+ Partials      474      470       -4
Impacted Files Coverage Δ
src/borg/archiver.py 81.36% <100%> (+0.11%) ⬆️
src/borg/helpers/parseformat.py 89% <0%> (+0.16%) ⬆️
src/borg/archive.py 83.12% <0%> (+0.61%) ⬆️

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 73ab1f1...1d9b4d0. Read the comment docs.

Copy link
Copy Markdown
Member

@ThomasWaldmann ThomasWaldmann left a comment

Choose a reason for hiding this comment

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

Is a bit hard to review due to the code moved around, but I assume that most stuff is just moved and has no changes?

Other than that, it looks good.

Did you test the manpage creation via setup.py?

Did you also check borg --help and borgfs --help?

group.add_argument('--last', metavar='N', dest='last', default=0, type=positive_int_validator,
help='consider last N archives after other filters were applied')

def define_borg_mount(subparser):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

guess this should be parser.

that the parser can be in fact a subparser sometimes is a detail not needed here (and also is not true for borgfs).

Comment on lines +2687 to +2688
subparser = parser
define_borg_mount(subparser)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

replace these 2 lines by:

define_borg_mount(parser)

The introduction of the subparser name is a relict from the old code structure - but as you have it as a function paramenter now, it is not needed any more.

@fantasya-pbem
Copy link
Copy Markdown
Contributor

Oh dear, guess that is my fault, the fix for issue #4471 was mine...
I can confirm that build_man now builds successful.

@Gu1nness
Copy link
Copy Markdown
Contributor Author

Indeed most of the patch is moving the code around.
I patched your remarks, I was tired when I wrote this it seems :)

@ThomasWaldmann ThomasWaldmann merged commit 5ce9024 into borgbackup:master Oct 19, 2019
@ThomasWaldmann
Copy link
Copy Markdown
Member

Thanks!

Is this change only needed for master or also for 1.1-maint?

@ThomasWaldmann
Copy link
Copy Markdown
Member

Also, see #4789, there might be another issue.

@Gu1nness
Copy link
Copy Markdown
Contributor Author

It does not seem to be necessary for 1.1-maint the man pages build correctly.
For #4789 the man page does not seem to list the commands especially (except in the "SEE ALSO" section, in which they are alphabetically unordered (but logically ordered :

borg-init(1), borg-create(1), borg-mount(1),  borg-extract(1),  borg-list(1),  borg-info(1),  borg-delete(1),  borg-prune(1), borg-recreate(1

@ThomasWaldmann
Copy link
Copy Markdown
Member

1.1-maint: ok, good.

what i meant with #4789 issue is that the borg.1 manpage is not updated.

@Gu1nness
Copy link
Copy Markdown
Contributor Author

Oh yeah indeed.
Having a look at the code, I can see here that the date is hardcoded in the man_intro.
Let me patch this on my fork and I'll give you the corresponding commit :)

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.

master: python setup.py build_man is broken

4 participants