Skip to content
This repository has been archived by the owner on Nov 19, 2020. It is now read-only.

test gel service #46

Merged
merged 2 commits into from
Nov 5, 2019
Merged

test gel service #46

merged 2 commits into from
Nov 5, 2019

Conversation

kylebrandt
Copy link
Contributor

No description provided.

kylebrandt
kylebrandt commented Nov 5, 2019
View reviewed changes
pkg/gelpoc/service_test.go Outdated Show resolved Hide resolved
aknuds1
aknuds1 reviewed Nov 5, 2019
View reviewed changes
Copy link
Contributor

@aknuds1 aknuds1 left a comment

Choose a reason for hiding this comment

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

There are a couple of error checks in the test, which I think should be converted into require.NoError.

pkg/gelpoc/service_test.go Outdated Show resolved Hide resolved
pkg/gelpoc/service_test.go Outdated Show resolved Hide resolved
marcusolsson
marcusolsson previously approved these changes Nov 5, 2019
View reviewed changes
Copy link
Contributor

@marcusolsson marcusolsson left a comment

Choose a reason for hiding this comment

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

Nice! Only comment I have is whether utp and fp should have more descriptive names.

@kylebrandt kylebrandt marked this pull request as ready for review November 5, 2019 16:16
aknuds1
aknuds1 approved these changes Nov 5, 2019
View reviewed changes
Copy link
Contributor

@aknuds1 aknuds1 left a comment

Choose a reason for hiding this comment

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

LGTM

@kylebrandt kylebrandt merged commit ba8b1fc into master Nov 5, 2019
@kylebrandt kylebrandt mentioned this pull request Nov 5, 2019
Closed
@kylebrandt kylebrandt deleted the test_gel_svc branch November 18, 2019 20:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@aknuds1 aknuds1 aknuds1 approved these changes

@marcusolsson marcusolsson marcusolsson left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@kylebrandt @aknuds1 @marcusolsson