Skip to content
This repository was archived by the owner on Jul 22, 2024. It is now read-only.

Return descriptive and comparable errors#6

Merged
mitchellh merged 3 commits intomitchellh:masterfrom
jghiloni:master
Nov 18, 2020
Merged

Return descriptive and comparable errors#6
mitchellh merged 3 commits intomitchellh:masterfrom
jghiloni:master

Conversation

@jghiloni
Copy link
Copy Markdown

There are situations in my use case where I need to be able to tell if Get is erroring because it can't find the key (a case I want to ignore) or for some other reason (that I do not want to ignore).

I've developed this in such a way that no error messages have changed and all existing tests pass. This should introduce no changes to how folks are using it now if they don't want to change.

@jghiloni
Copy link
Copy Markdown
Author

It's the same except now you need Go 1.13+ to test 😬

@jghiloni
Copy link
Copy Markdown
Author

jghiloni commented Aug 24, 2020

do we need to test as far back as 1.7?

Edit: i think i have a fix in mind, I'll add it

The code has been refactored that pre-go1.13, the behavior is
identical to the way it was before. Tested on go1.15, 1.12
(reverted the updated go.mod to its original state), and 1.7.6
Copy link
Copy Markdown
Owner

@mitchellh mitchellh left a comment

Choose a reason for hiding this comment

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

This looks good! I don't think we need to support before Go 1.13 so if you want to revert that commit I'd be fine bumping the requirements up.

@jghiloni
Copy link
Copy Markdown
Author

Well I've got everything working now way far back to 1.7, so happy to leave it as is for any legacy users

@jghiloni
Copy link
Copy Markdown
Author

will leave that choice up to you, happy to revert if you'd rather that for code cleanliness

@jghiloni
Copy link
Copy Markdown
Author

@mitchellh I will revert that commit and bump your travis.yml so it's only testing 1.13 and tip

@jghiloni
Copy link
Copy Markdown
Author

Hi @mitchellh! Has there been any movement on this? Thanks!

@jghiloni
Copy link
Copy Markdown
Author

Hi folks! This sure would be nice to have, any chance we could get it merged in?

@mitchellh
Copy link
Copy Markdown
Owner

Sorry I dropped this! Merging.

@mitchellh mitchellh merged commit a15bd03 into mitchellh:master Nov 18, 2020
@jghiloni
Copy link
Copy Markdown
Author

thanks! 🙏

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants