-
Notifications
You must be signed in to change notification settings - Fork 167
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
Release 3.4.2 preparation #630
Conversation
@@ -94,7 +94,7 @@ def modify_xml(xml_path, info): | |||
background_path = join(OSX_DIR, 'MacInstaller.png') | |||
|
|||
if background_path: | |||
logger.info("Using background image", background_path) | |||
logger.info("Using background image: %s", background_path) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this not caught by any linter? I'm surprised 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, me too. Seems like I previously only inspected the linux/windows logs and flake8 seems to not catch it (I am more used to pylint, which seems too be able to catch it... pylint is generally stricter, but also supports different code styles and options so that is hard to find an agreement what to set... flake8 is like the baseline linter, not so strict, default options are sane, extendable via extension)
After I found this one occurence, I inspected the full logs via printing stdout/stderr also in succeeding tests in my fork of the repo (where the CI was not clogged). Those logs are awesome now: you see how constructor runs start in main, continue in e.g. osxpkg, utils, fcp, back to osxpkg until it finishes etc ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is good to go! Thanks Daniel!
Description
closes #629
Be carefully reviewing this, it contains more than just changelog updates: During preparing the release I looked through our Github CI runs once again, inspected all the CI logs, downloaded the created installers and inspected them. I found one logging error + a documentation error. Both are fixed by this PR including Changelog update! See the last commits in this PR. I can split this out into a separate PR, but that is no fun, given the current clogged GitHub Action runners.
Checklist - did you ...
news
directory (using the template) for the next release's release notes?