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.

Sign up for GitHub

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

Jump to bottom

[JUJU-1610] Replace kr/pretty with canonical/pretty #14493

Merged
merged 1 commit into from
Aug 22, 2022
Merged

[JUJU-1610] Replace kr/pretty with canonical/pretty #14493

merged 1 commit into from
Aug 22, 2022

Conversation

barrettj12
Copy link
Contributor

This pulls through a fix for issue kr/pretty#77 which was causing some of our logs to format like this:

[LOG] 0:00.001 DEBUG juju.kubernetes.provider.application creating storage class for gitlab filesystem database: &v1.StorageClass{
%!v(PANIC=Format method: runtime error: invalid memory address or nil pointer dereference)
[LOG] 0:00.001 DEBUG juju.kubernetes.provider.application using persistent volume claim for gitlab filesystem database: v1.PersistentVolumeClaim{
%!v(PANIC=Format method: runtime error: invalid memory address or nil pointer dereference)

We've pushed the patch upstream, but kr/pretty doesn't look actively maintained anymore. Until they accept our patch, we will use our fork at canonical/pretty instead.

QA steps

go test -v ./caas/kubernetes/provider/application -check.f applicationSuite.TestEnsureStateful -check.vv

Verify that the structs are printed correctly in the logs.

@barrettj12 barrettj12 added the do not merge Even if a PR has been approved, do not merge the PR! label Aug 22, 2022
@barrettj12 barrettj12 changed the base branch from develop to 2.9 August 22, 2022 00:06
@barrettj12 barrettj12 added dependencies Pull requests that update a dependency file 2.9 and removed do not merge Even if a PR has been approved, do not merge the PR! labels Aug 22, 2022
benhoyt
benhoyt approved these changes Aug 22, 2022
View reviewed changes
Copy link
Member

@benhoyt benhoyt left a comment

Choose a reason for hiding this comment

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

Thanks!

@barrettj12
Copy link
Contributor Author

/merge

@jujubot jujubot merged commit 6eedc6f into juju:2.9 Aug 22, 2022
@barrettj12 barrettj12 deleted the kr-pretty-replace branch August 22, 2022 02:12
@ycliuhw ycliuhw mentioned this pull request Aug 22, 2022
Merged
@juanmanuel-tirado
Copy link
Contributor

It would be better to have a 0.3.1 release of canonical/pretty

jujubot added a commit that referenced this pull request Aug 23, 2022
@jujubot
Merge pull request #14499 from ycliuhw/2.9-into-develop
874e170 
#14499

```
Merge branch '2.9' into 2.9-into-develop

# Conflicts:
# api/agent/uniter/unit.go
# go.mod
# go.sum
# tests/suites/agents/key_workers_run.sh
# tests/suites/hooks/dispatch.sh
# tests/suites/smoke/deploy.sh

```

- #14478 
- #14484
- #14493
- #14496
- #14497
- #14495
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benhoyt benhoyt benhoyt approved these changes

Assignees
No one assigned
Labels
2.9 dependencies Pull requests that update a dependency file
Projects
None yet
Milestone
No milestone
4 participants
@barrettj12 @juanmanuel-tirado @benhoyt @jujubot