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

fix: error readable by using the right placeholder #39

Merged
merged 2 commits into from
Nov 25, 2022

Conversation

kokes
Copy link
Contributor

@kokes kokes commented Nov 23, 2022

Description of changes:

Logs aren't as readable as they could be, I saw this during finch vm init:

INFO[0009] sudoers file not found: %!w(*fs.PathError=&{open /etc/sudoers.d/finch-lima 2})

From the looks of it, we don't need the %w since the current logging implementation doesn't use fmt.Errorf under the hood anyway, and thus makes the output hard to read without the only potential benefit being utilised (unwrapping). And since the logging interface's methods themselves don't return anything, it's fairly likely unwrapping won't happen in any other implementation (there isn't even a reason for it anyway - the callee will have all the args available to it, so it can introspect the error passed directly).

(Since this Infof call is made right after an errors.Is, the whole %v/%w bit is redundant as it will just say "sudoers file not found: no such file or directory", but I didn't want to change error semantics in this PR.)

Testing done:

make test-unit and using logrus directly to verify none of its *f methods use fmt.Errorf.

  • I've reviewed the guidance in CONTRIBUTING.md

License Acceptance

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@ningziwen ningziwen requested a review from pendo324 November 23, 2022 20:16
Copy link
Contributor

@davidhsingyuchen davidhsingyuchen left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! LGTM. Could you fix the DCO check by git commit -s --amend --no-edit and then push -f?

@davidhsingyuchen davidhsingyuchen self-assigned this Nov 24, 2022
@kokes kokes force-pushed the error_placeholders branch from 71e866c to c124118 Compare November 24, 2022 06:38
@kokes
Copy link
Contributor Author

kokes commented Nov 24, 2022

Could you fix the DCO check

Oh, right, done.

@kokes kokes mentioned this pull request Nov 24, 2022
1 task
@davidhsingyuchen davidhsingyuchen merged commit 8e5f38d into runfinch:main Nov 25, 2022
sam-berning pushed a commit that referenced this pull request Dec 6, 2022
🤖 I have created a release *beep* *boop*
---


## [0.1.1](v0.1.0...v0.1.1)
(2022-12-06)


### Bug Fixes

* added the contrib folder to be ignored by CI
([#92](#92))
([3415f2a](3415f2a))
* error readable by using the right placeholder
([#39](#39))
([8e5f38d](8e5f38d))


### Build System or External Dependencies

* **deps:** bump finch-core to 0.1.1
([#93](#93))
([3f3bce5](3f3bce5))
* **deps:** Bump github.com/lima-vm/lima from 0.12.0 to 0.13.0
([#40](#40))
([520cc7f](520cc7f))
* **deps:** Bump github.com/onsi/ginkgo/v2 from 2.5.0 to 2.5.1
([#50](#50))
([fa108fd](fa108fd))
* **deps:** Bump github.com/runfinch/common-tests version from v0.1.0 to
v0.1.1 ([#76](#76))
([fd22d4a](fd22d4a))
* **deps:** Bump github.com/spf13/afero from 1.9.2 to 1.9.3
([#43](#43))
([bf0ad84](bf0ad84))
* **deps:** Bump github.com/stretchr/testify from 1.8.0 to 1.8.1
([#44](#44))
([31c6d70](31c6d70))
* **deps:** Bump github.com/xorcare/pointer from 1.2.1 to 1.2.2
([#42](#42))
([8e83137](8e83137))
* **deps:** Bump golang.org/x/crypto from 0.1.0 to 0.3.0
([#49](#49))
([89826cf](89826cf))
* **deps:** Bump golang.org/x/tools from 0.2.0 to 0.3.0
([#52](#52))
([27c8f24](27c8f24))
* **deps:** Bump k8s.io/apimachinery from 0.25.2 to 0.25.4
([#51](#51))
([8f15779](8f15779))
* **deps:** Bump lima version
([#75](#75))
([cfaa4f6](cfaa4f6))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
ahsan-z-khan pushed a commit to ahsan-z-khan/finch that referenced this pull request Jan 11, 2023
🤖 I have created a release *beep* *boop*
---


## [0.1.1](runfinch/finch@v0.1.0...v0.1.1)
(2022-12-06)


### Bug Fixes

* added the contrib folder to be ignored by CI
([runfinch#92](runfinch#92))
([3415f2a](runfinch@3415f2a))
* error readable by using the right placeholder
([runfinch#39](runfinch#39))
([8e5f38d](runfinch@8e5f38d))


### Build System or External Dependencies

* **deps:** bump finch-core to 0.1.1
([runfinch#93](runfinch#93))
([3f3bce5](runfinch@3f3bce5))
* **deps:** Bump github.com/lima-vm/lima from 0.12.0 to 0.13.0
([runfinch#40](runfinch#40))
([520cc7f](runfinch@520cc7f))
* **deps:** Bump github.com/onsi/ginkgo/v2 from 2.5.0 to 2.5.1
([runfinch#50](runfinch#50))
([fa108fd](runfinch@fa108fd))
* **deps:** Bump github.com/runfinch/common-tests version from v0.1.0 to
v0.1.1 ([runfinch#76](runfinch#76))
([fd22d4a](runfinch@fd22d4a))
* **deps:** Bump github.com/spf13/afero from 1.9.2 to 1.9.3
([runfinch#43](runfinch#43))
([bf0ad84](runfinch@bf0ad84))
* **deps:** Bump github.com/stretchr/testify from 1.8.0 to 1.8.1
([runfinch#44](runfinch#44))
([31c6d70](runfinch@31c6d70))
* **deps:** Bump github.com/xorcare/pointer from 1.2.1 to 1.2.2
([runfinch#42](runfinch#42))
([8e83137](runfinch@8e83137))
* **deps:** Bump golang.org/x/crypto from 0.1.0 to 0.3.0
([runfinch#49](runfinch#49))
([89826cf](runfinch@89826cf))
* **deps:** Bump golang.org/x/tools from 0.2.0 to 0.3.0
([runfinch#52](runfinch#52))
([27c8f24](runfinch@27c8f24))
* **deps:** Bump k8s.io/apimachinery from 0.25.2 to 0.25.4
([runfinch#51](runfinch#51))
([8f15779](runfinch@8f15779))
* **deps:** Bump lima version
([runfinch#75](runfinch#75))
([cfaa4f6](runfinch@cfaa4f6))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

3 participants