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

Add account name to pubsub and bigquery #699

Merged
merged 6 commits into from
Nov 21, 2022

Conversation

grantstephens
Copy link
Contributor

@grantstephens grantstephens commented Nov 1, 2022

This is blocked waiting for a new release of go-fastly. The tests need to be updated, but it cannot be tested without the release, so waiting on that.

@Integralist
Copy link
Collaborator

@grantstephens there's been a v7 release now so you can continue on with this PR.

@grantstephens grantstephens force-pushed the grantstephens/add-account-name-google branch from 179812d to d073f7b Compare November 21, 2022 10:22
@grantstephens grantstephens force-pushed the grantstephens/add-account-name-google branch from d345623 to ebb899a Compare November 21, 2022 11:30
Copy link
Collaborator

@Integralist Integralist left a comment

Choose a reason for hiding this comment

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

LGTM. Just a couple of small tweaks...

@@ -366,7 +370,7 @@ func getBigQueryError(i *fastly.GetBigQueryInput) (*fastly.BigQuery, error) {
return nil, errTest
}

var describeBigQueryOutput = "\n" + strings.TrimSpace(`
var describeBigQueryOutput = "\n" + strings.TrimSpace(`Account name: `+`
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this can be simplified to:

Suggested change
var describeBigQueryOutput = "\n" + strings.TrimSpace(`Account name: `+`
var describeBigQueryOutput = "\n" + strings.TrimSpace(`Account name:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes and no- depending on your editor it removes the space after account name and then the test fails- this is a hacky way around it- the other way is to set it to an actual value so that it isn't a blank line./

Copy link
Contributor Author

@grantstephens grantstephens Nov 21, 2022

Choose a reason for hiding this comment

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

Fixed in 4e0a709
Sorry, forgot I'd done this in trying to get the tests working

pkg/commands/logging/common/flags.go Outdated Show resolved Hide resolved
@Integralist Integralist merged commit 4dfc0ac into main Nov 21, 2022
@Integralist Integralist deleted the grantstephens/add-account-name-google branch November 21, 2022 15:56
@Integralist Integralist added the enhancement New feature or request label Nov 29, 2022
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