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

CI fixes #578

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

CI fixes #578

wants to merge 14 commits into from

Conversation

emilsvennesson
Copy link
Owner

No description provided.

@emilsvennesson emilsvennesson changed the title pylint fixes CI fixes Oct 5, 2024
@emilsvennesson emilsvennesson force-pushed the pylint branch 4 times, most recently from 6aa2e4b to 8b29498 Compare October 5, 2024 23:32
Copy link

sonarqubecloud bot commented Oct 6, 2024

Quality Gate Passed Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

Copy link
Collaborator

@horstle horstle left a comment

Choose a reason for hiding this comment

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

All in all some nice improvements.

"""Perform an HTTP request and return request"""
log(0, 'Request URL: {url}', url=url)

def _http_request(url, headers=None, time_out=30):
Copy link
Collaborator

Choose a reason for hiding this comment

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

the headers parameter would be unused after this change

except (HTTPError, URLError) as err:
log(2, 'Download failed with error {}'.format(err))
if yesno_dialog(localize(30004), '{line1}\n{line2}'.format(line1=localize(30063), line2=localize(30065))): # Internet down, try again?
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please leave this comment in, so one doesn't have to look up what message 30065 says.

if req is None:
return None
continue
while True:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I try to avoid while True. To make sure we can never get stuck in an infinite loop, I would stick with while size < total_length

time_left = int(round((total_length - size) * (time() - starttime) / size))
prog_message = '{line1}\n{line2}'.format(
line1=message,
line2=localize(30058, mins=time_left // 60, secs=time_left % 60)) # Time remaining
Copy link
Collaborator

Choose a reason for hiding this comment

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

Again, keep the comment. I don't want to have to look up what message 30058 is.

cdm_versions = http_get(config.WIDEVINE_VERSIONS_URL)
log(0, f"Available Widevine versions from repo: {cdm_versions}")
cdm_versions = cdm_versions.strip('\n').split('\n')
log(0, f"Available Widevine versions from repo: {cdm_versions}")
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is a bit much, even in a debug log

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.

2 participants