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

Update FTP logging endpoint #165

Merged
merged 11 commits into from
May 12, 2020
Merged

Update FTP logging endpoint #165

merged 11 commits into from
May 12, 2020

Conversation

ezkl
Copy link
Contributor

@ezkl ezkl commented May 6, 2020

  • Rename Username to User for consistency with API field
  • Add PublicKey
  • Use pointers in CreateFTPInput and UpdateFTPInput structs and use helpers in tests
  • Change GzipLevel from uint8 to uint for consistency

(edits by @mccurdyc)

  • Rename Username to User for consistency with API field
  • Add PublicKey
  • Use pointers in CreateFTPInput and UpdateFTPInput structs and use helpers in tests
  • Change GzipLevel from uint8 to uint for consistency

The crossed-out fields are breaking changes and need to wait for v2.

@ezkl ezkl requested a review from phamann May 6, 2020 22:23
@ezkl ezkl changed the title Add FTP logging endpoint support Update FTP logging endpoint May 6, 2020
* Rename FTP.Username => FTP.User for consistency
* Add FTP.PublicKey
* Pointers in CreateFTPInput, UpdateFTPInput and use helpers in tests
* Change GzipLevel from uint8 to uint for consistency
@mccurdyc
Copy link
Contributor

mccurdyc commented May 11, 2020

@phamann and I talked about this. While this is what we would like the API to be eventually (e.g., pointers, etc.), we can't make some of these changes right now without releasing a major version of the client, which we aren't ready to do.

So, for this change, we will focus on additions to the API, but no in-place changes and will note for these changes to be made in a v2 release of the client.

mccurdyc added 2 commits May 12, 2020 11:29
Signed-off-by: Colton McCurdy <cmccurdy@fastly.com>
Signed-off-by: Colton McCurdy <cmccurdy@fastly.com>
Signed-off-by: Colton McCurdy <cmccurdy@fastly.com>
@@ -169,6 +176,14 @@ func TestClient_FTPs(t *testing.T) {
if uftp.Name != "new-test-ftp" {
t.Errorf("bad name: %q", uftp.Name)
}
// TODO (v2): This is a bug where updates to zero-values are omitted due to the
Copy link
Contributor

Choose a reason for hiding this comment

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

@phamann test that exercises the bug with a note for v2.

Signed-off-by: Colton McCurdy <cmccurdy@fastly.com>
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 other than the pointer comment

mccurdyc added 5 commits May 12, 2020 11:49
Signed-off-by: Colton McCurdy <cmccurdy@fastly.com>
Signed-off-by: Colton McCurdy <cmccurdy@fastly.com>
Signed-off-by: Colton McCurdy <cmccurdy@fastly.com>
Signed-off-by: Colton McCurdy <cmccurdy@fastly.com>
@mccurdyc mccurdyc requested a review from phamann May 12, 2020 15:56
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

@mccurdyc mccurdyc merged commit c2c2c62 into fastly:master May 12, 2020
@ezkl ezkl deleted the ezkl/add-ftp branch May 19, 2020 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants