Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Refactor dashboard utils #370

Merged
merged 7 commits into from
Jan 10, 2025
Merged

Refactor dashboard utils #370

merged 7 commits into from
Jan 10, 2025

Conversation

sed-i
Copy link
Contributor

@sed-i sed-i commented Jan 9, 2025 •
edited
Loading

This PR is a small, scoped change, extracted from #363.
This way #363 will be easier to review, and the changes in this PR are trivial.

  • Use cosl.LZMABase64 instead of _encode_dashboard_content, _decode_dashboard_content.
  • Namespace the following free functions under class CharmedDashboard. Github's diff algorithm produces a messy diff. In the attached diff (report.pdf) it's easier to see that the difference is just indentation.
    • _convert_dashboard_fields
    • _replace_template_fields
    • _template_panels
    • _inject_labels
    • _modify_panel

@sed-i sed-i requested a review from a team as a code owner January 9, 2025 20:29
@sed-i sed-i changed the title Feature/dashlib refactor Refactor dashboard utils Jan 9, 2025
MichaelThamm
MichaelThamm approved these changes Jan 9, 2025
View reviewed changes
Copy link
Contributor

@MichaelThamm MichaelThamm left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I like the CharmedDashboard class for cleanliness, approved on conditional itest success

Abuelodelanada
Abuelodelanada approved these changes Jan 10, 2025
View reviewed changes
Copy link
Contributor

@Abuelodelanada Abuelodelanada left a comment

Choose a reason for hiding this comment

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

LGTM!

@sed-i sed-i merged commit 95c2e04 into main Jan 10, 2025
13 checks passed
@sed-i sed-i deleted the feature/dashlib-refactor branch January 10, 2025 20:06
This was referenced Jan 15, 2025
Merged
Merged
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@Abuelodelanada Abuelodelanada Abuelodelanada approved these changes

@MichaelThamm MichaelThamm Awaiting requested review from MichaelThamm

Assignees
No one assigned
Labels
Libraries: Out of sync
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sed-i @Abuelodelanada @MichaelThamm