Merge ~logrotate-charmers/charm-logrotated:bugs_1833095_1833093 into charm-logrotated:master

Proposed by Jeremy Lounder
Status: Merged
Approved by: Paul Goins
Approved revision: 6d391ef597fad5552bfdee132025c4aa603a21d9
Merged at revision: b2e03efd1888e6644d178af2f5f098ee326e9df5
Proposed branch: ~logrotate-charmers/charm-logrotated:bugs_1833095_1833093
Merge into: charm-logrotated:master
Diff against target: 674 lines (+151/-132)
10 files modified
actions/actions.py (+12/-21)
dev/null (+0/-13)
lib/lib_cron.py (+38/-32)
lib/lib_logrotate.py (+20/-25)
reactive/logrotate.py (+20/-8)
tests/functional/conftest.py (+14/-9)
tests/functional/juju_tools.py (+14/-13)
tests/functional/test_logrotate.py (+5/-2)
tests/unit/conftest.py (+11/-0)
tests/unit/test_logrotate.py (+17/-9)
Reviewer Review Type Date Requested Status
Paul Goins Approve
Canonical IS Reviewers Pending
Review via email: mp+378375@code.staging.launchpad.net

Commit message

LP#1833095 LP#1833093 Fixes linting and unit test issues.

To post a comment you must log in.
Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

Unable to determine commit message from repository - please click "Set commit message" and enter the commit message manually.

Revision history for this message
Paul Goins (vultaire) wrote :

Looks pretty good. I've suggested a few small improvements, although functionally the code is fine as-is without them. However, removal of the copyright blocks concerns me - I think those need to be restored.

review: Needs Fixing
Revision history for this message
Paul Goins (vultaire) wrote :

Adding one more comment

Revision history for this message
Paul Goins (vultaire) wrote :

Created bug regarding this code review: https://pad.lv/1864572

As this code is already merged into the current released charm, am merging this as-is.

review: Approve

There was an error fetching revisions from git servers. Please try again in a few minutes. If the problem persists, contact Launchpad support.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: