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

Migrate to Go-Fastly 2.0.0 #169

Merged
merged 13 commits into from
Nov 24, 2020
Merged

Conversation

Integralist
Copy link
Collaborator

Problem: multiple lines of tagged releases for Go-Fastly is making adding new features via our various API clients much harder.
Solution: migrate cli to using Go-Fastly 2.0.0

@Integralist Integralist requested a review from phamann November 18, 2020 22:04
@Integralist Integralist force-pushed the integralist/2020_11_18_go_fastly_2.0.0 branch 2 times, most recently from c236e6c to fe74ffa Compare November 20, 2020 18:18
@Integralist Integralist marked this pull request as ready for review November 20, 2020 18:18
@Integralist
Copy link
Collaborator Author

@phamann this is ready for you (see DM also).

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 - Thank you so much for this very tedious, but very important upgrade!

My only comments are minor and non-blocking:

  • A personal preference, but naming of optional.WasSet seems weird to me and makes the type less portable for use outside of flag actions. IsValid or HasValue may have been better choices.
  • I formatted lots of comments per: https://github.com/golang/go/wiki/CodeReviewComments#comment-sentences. Most of them were inline and thus not an issue, but I prefer the consistency.
  • Would be good to change the comment fields in go-fastly asap so they don't become breaking changes.

pkg/api/interface.go Outdated Show resolved Hide resolved
pkg/domain/update.go Outdated Show resolved Hide resolved
pkg/service/update.go Outdated Show resolved Hide resolved
pkg/service/update.go Outdated Show resolved Hide resolved
pkg/service/update.go Outdated Show resolved Hide resolved
pkg/serviceversion/update.go Outdated Show resolved Hide resolved
pkg/serviceversion/update.go Outdated Show resolved Hide resolved
pkg/serviceversion/update.go Outdated Show resolved Hide resolved
@phamann phamann added the enhancement New feature or request label Nov 24, 2020
@phamann phamann force-pushed the integralist/2020_11_18_go_fastly_2.0.0 branch from 46c375c to e3869bf Compare November 24, 2020 14:06
@phamann phamann merged commit 782b9ed into master Nov 24, 2020
@phamann phamann deleted the integralist/2020_11_18_go_fastly_2.0.0 branch November 24, 2020 14:48
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