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

Code comments and PEP8 prettification. #1360

Merged
merged 7 commits into from
Nov 12, 2022
Merged

Code comments and PEP8 prettification. #1360

merged 7 commits into from
Nov 12, 2022

Conversation

buhtz
Copy link
Member

@buhtz buhtz commented Nov 9, 2022

While investiagting the manpage creation script some comments where added. Also some code formatting was done to improve readability and better fitting PEP8 rules.

Also repaired special #? comments accidentially removed in PR #1313 before.

Related to #1354

While investiagting the manpage creation script some comments where
added. Also some code formatting was done to improve readability and
better fitting PEP8 rules.

Also repaired special #? comments accidentially removed in PR #1313
before.

Related to #1354
common/config.py Outdated Show resolved Hide resolved
@emtiu
Copy link
Member

emtiu commented Nov 9, 2022

Some minor suggestions, but looks good otherwise :)

Out of curiosity: Is the manpage generation covered by testing somehow? Or does it only happen when we run the script manually?

@buhtz
Copy link
Member Author

buhtz commented Nov 9, 2022

Out of curiosity: Is the manpage generation covered by testing somehow?

No.

Or does it only happen when we run the script manually?

Only, when we run it manually. It is a "helper" script and conceptually don't belong to the release version of BIT. But looking into my Debian installation of BIT that py file is shipped with the release version.

@aryoda
Copy link
Contributor

aryoda commented Nov 9, 2022

I think my fix for #1270 (schedule.time doc outdated) is still lost if we create the man pages again.

My changes from 3c1e08e need to be added here:

backintime/common/config.py

Lines 852 to 855 in 775c9c3

def scheduleTime(self, profile_id = None):
#?What time the cronjob should run? Only valid for
#?\fIprofile<N>.schedule.mode\fR >= 20;0-24
return self.profileIntValue('schedule.time', 0, profile_id)

Shall I commit to your PR or could you do the change for me (saves me the setup overhead ;-) ?

@buhtz
Copy link
Member Author

buhtz commented Nov 10, 2022

Shall I commit to your PR or could you do the change for me (saves me the setup overhead ;-) ?

You mean this?
#1270 (comment)

I'll do it for you. ❤️

@aryoda
Copy link
Contributor

aryoda commented Nov 12, 2022

Can we merge this?

@buhtz buhtz merged commit 34359ca into bit-team:master Nov 12, 2022
@buhtz buhtz deleted the enh/1354 branch November 12, 2022 12:40
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