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

Create new error abstraction for field validation #239

Merged
merged 29 commits into from
Jan 19, 2021

Conversation

Integralist
Copy link
Collaborator

@Integralist Integralist commented Dec 17, 2020

Create new error abstraction for field validation. This helps with consistency and maintainability.

@Integralist Integralist force-pushed the integralist/2020_11_19_missed_refs branch 2 times, most recently from f93a1be to 7a28525 Compare December 21, 2020 14:35
README.md Show resolved Hide resolved
@Integralist Integralist force-pushed the integralist/2020_11_19_missed_refs branch 2 times, most recently from 16eb5b9 to 7b8e719 Compare December 21, 2020 14:51
@Integralist Integralist marked this pull request as ready for review December 26, 2020 21:20
README.md Show resolved Hide resolved
go.mod Show resolved Hide resolved
Copy link
Member

@phamann phamann left a comment

Choose a reason for hiding this comment

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

Looks good, other than my comment around error constants, let me know if you want to chat about the best pattern before changing anything.

fastly/errors.go Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
@Integralist Integralist force-pushed the integralist/2020_11_19_missed_refs branch 4 times, most recently from 6e2d33e to d0140e5 Compare January 5, 2021 16:56
@Integralist
Copy link
Collaborator Author

@phamann changes made, waiting re-review

Copy link
Member

@phamann phamann left a comment

Choose a reason for hiding this comment

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

👍🏻 LGTM

Will you be incorporating the other PR into this release?

@Integralist
Copy link
Collaborator Author

@phamann yes. the author replied to me eariler to say they would take a look next week.

So with that in mind. I can merge this change but not cut a release until after they've managed to wrap up any other changes and write some tests in their PR.

@Integralist Integralist added the enhancement New feature or request label Jan 14, 2021
@Integralist Integralist force-pushed the integralist/2020_11_19_missed_refs branch from d0140e5 to 33c924a Compare January 19, 2021 16:01
@Integralist Integralist changed the title Go-Fastly Tweaks Create new error abstraction for field validation Jan 19, 2021
@Integralist Integralist force-pushed the integralist/2020_11_19_missed_refs branch from cfabd53 to 72e149f Compare January 19, 2021 16:50
@Integralist Integralist force-pushed the integralist/2020_11_19_missed_refs branch from e1c3545 to dc644df Compare January 19, 2021 17:05
@Integralist
Copy link
Collaborator Author

@fastly/developer-relations @phamann I've reverted a bunch of approved changes (see the latest commits above). I'm going to open a few separate PRs to home these changes (as they've already been approved by Patrick they should be quick to review/approve) but it'll help ensure the dynamically generated CHANGELOG will have explicit callouts to each change (whereas if I kept them all within this PR they would be lost).

@Integralist Integralist merged commit ee7e2bb into master Jan 19, 2021
@Integralist Integralist deleted the integralist/2020_11_19_missed_refs branch January 19, 2021 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants