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

Merge Upstream #3

Merged
merged 11 commits into from
Jan 10, 2024
Merged

Merge Upstream #3

merged 11 commits into from
Jan 10, 2024

Conversation

mcDevnagh
Copy link

and fix merge conflicts

mmorel-35 and others added 5 commits August 4, 2021 17:13
This updates github.com/rogpeppe/go-internal from 1.6.1 to 1.8.0
and the github action 'setup-go' from v1 to v2.

It also adds a config file for dependabot to automatically check
this repo for new dependency versions weekly.
Bumps [actions/checkout](https://github.com/actions/checkout) from 2 to 3.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@v2...v3)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
This handles a few cases (similar to how fmt %#v does):

- A GoString method on a value receiver, called with a nil pointer
- A GoString method on a pointer receiver that doesn't check for nil
- A GoString method that panics in some other way

Because Go 1.17 added a method Time.GoString with value receiver, this
broke structs that had *time.Time fields with nil values (which is
common!).

Also added a bunch of tests for these cases.

Fixes kr#77

Co-authored-by: Jordan Barrett <[email protected]>
This change bumps github.com/rogpeppe/go-internal from version 1.8.0 to 1.9.0.

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@mcDevnagh mcDevnagh mentioned this pull request Jan 5, 2024
@tjex tjex self-requested a review January 5, 2024 10:57
Sync with master first to refresh go.mod / go.sum and run tests again,
before merging PR.
Copy link
Member

@tjex tjex left a comment

Choose a reason for hiding this comment

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

@mcDevnagh the build fails on GitHub CI, but go test -v passes fine locally for me on your kr-main branch.

Is this expected? Is it something that gets resolved after merge because of the github.com/zk-org/pretty url change?

Or is it something we should fix here before merging to zk-org main?

Otherwise everything is looking fine.

@tjex
Copy link
Member

tjex commented Jan 10, 2024

Hey @mickael-menu . Pinging you on this one, just to double check that we're handling this the right way.
Would be great to make sure we're not overlooking anything during the transferal of this dependancy to our own managed fork.

@mickael-menu
Copy link
Member

Have you tried bumping the Go version in the CI workflow? I think that might impact the output.

But I think it's fine if you prefer to use directly the upstream pretty. I mainly forked it to improve the quality of life when analyzing failing tests.

@tjex
Copy link
Member

tjex commented Jan 10, 2024

Yup that did it :) Thanks again!

@tjex tjex requested review from tjex and removed request for mickael-menu January 10, 2024 20:34
@tjex tjex merged commit 7ca3601 into zk-org:main Jan 10, 2024
1 check passed
@tjex tjex mentioned this pull request Jan 11, 2024
@mcDevnagh
Copy link
Author

thanks for handling this one gents

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.

5 participants