Reduce dependencies #734
Reduce dependencies #734
Conversation
|
A comment on d76ff69 and I came up with a custom implementation that fulfills our current usage of the library in order to try to comply with the previous API and log format. E:g.: Example of previous output by Hashicorps multierror library, which has been mimicked by our custom version: I think this is the safest bet before the release, but nevertheless in the future we should review this implementation and try to replace it with something simpler. For example go v1.20 adds support for wrapping multiple errors. |
There was a problem hiding this comment.
Thanks for doing this! Most of it looks good. I've left some suggestions that I think are worth considering, and let me know in the comments if my suggestions don't work 🙂
EDIT: Could you also double check the CI pipeline issues and resolve those? There are some tests that fail often in the CI pipeline (we have raised issues for those so you should be able to find them, and then rerun the failed tests runs).
ka3de
force-pushed
the
optimization/729-reduce-dependencies
branch
from
b4f3aad to
d76ff69
Compare
If the given inner logger for our log implementation is nil, provide a new logrus logger by default. This avoids a possible panic when checking for the log level in the Logf method as well as allows us to remove the particular logging action using Printf when wrapped logger was nil. Resolves: #734 (comment)
If the given inner logger for our log implementation is nil, provide a new logrus logger by default. This avoids a possible panic when checking for the log level in the Logf method as well as allows us to remove the particular logging action using Printf when wrapped logger was nil. Resolves: #734 (comment)
This dependency is now included as "indirect", but it's only required from the tests package, therefore should not be included in the generated binary.
ka3de
force-pushed
the
optimization/729-reduce-dependencies
branch
from
805bf89 to
8dcfbb6
Compare
This commit replaces the Hashicorp multierror dependency usage with a custom implementation which fulfills our use case. The implementation tries to comply with the error format used by the replaced library for grouping multiple errors. Co-authored-by: İnanç Gümüş <inanc.gumus@grafana.com>
Considering the merge with k6, this will allow the final generated binary to include a single version of the mentioned dependency.
ka3de
force-pushed
the
optimization/729-reduce-dependencies
branch
from
f6d1e5f to
6ec0732
Compare
This PR tries to get rid of some dependencies currently required by the main module that are marginally used. More specifically:
github.com/fatih/colorgolang.org/x/expfrom main module (still required as transitive)This produces the direct inclusion of
github.com/google/go-cmpbut that is only used insidetestspkggit.luolix.top/hashicorp/go-multierrorwith a custom implementation that fulfills our current use caseCloses #729.