theredditbandit / pman

A CLI project manager
Other
36 stars 3 forks source link

refactor ReadReadme func #62

Closed theredditbandit closed 2 months ago

ccoVeille commented 2 months ago

It's worth trying to refactor this

-return nil, errors.Join(ErrReadREADME, fmt.Errorf("something went wrong while reading README for %s: %w", projectName, err))
+return nil, fmt.Errorf("something went wrong while reading README %w for %s: %w", ErrReadREADME, projectName, err)
theredditbandit commented 2 months ago

It's worth trying to refactor this

-return nil, errors.Join(ErrReadREADME, fmt.Errorf("something went wrong while reading README for %s: %w", projectName, err))
+return nil, fmt.Errorf("something went wrong while reading README %w for %s: %w", ErrReadREADME, projectName, err)

could you elaborate on this , I'm not sure how to go about refactoring this or rather what is wrong with this piece and what I should be aiming for.

ccoVeille commented 2 months ago

could you elaborate on this , I'm not sure how to go about refactoring this or rather what is wrong with this piece and what I should be aiming for.

https://stackoverflow.com/questions/33470649/how-do-we-combine-multiple-error-strings-in-golang

I would find simple to read, but it's true that might be opinionated.

I looked for documentation and you might be right, when adding no wrapping information, using errors.Join might be a good way.

https://www.tiredsg.dev/blog/wrapping-multiple-errors-golang/

I would say that if you

It's just that for me, as you were already using fmt.Errorf("%w"), adding a second %w would have make more sense.

I hope it's clearer. But, feel free to ignore my idea.

theredditbandit commented 2 months ago

Thanks for the clear explanation, this makes sense.