Skip to content
This repository has been archived by the owner on Sep 30, 2024. It is now read-only.

Enable external services by default and update documentation #1342

Merged
merged 22 commits into from
Dec 12, 2018

Conversation

nicksnyder
Copy link
Contributor

@nicksnyder nicksnyder commented Dec 11, 2018

@nicksnyder nicksnyder added this to the 3.0-preview milestone Dec 12, 2018
}
}
return GQL.ExternalServiceKind.GITHUB
})(),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@felixfbecker I am going to merge this PR soon, but I would appreciate your review on this particular snippet. I need this component to respond to a url query parameter. This works, but I don't know if there is a more idiomatic way to do this.

Copy link
Contributor

Choose a reason for hiding this comment

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

The code LGTM, but this only gets evaluated on component instantiation and not when this.props.history.location.search changes. I don't see kind being changed anywhere, so I would recommend to remove this state field and call this as a function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nicksnyder nicksnyder merged commit ffbd6a3 into master Dec 12, 2018
@nicksnyder nicksnyder deleted the extsvcdoc branch December 12, 2018 22:35
@codecov-io
Copy link

Codecov Report

Merging #1342 into master will decrease coverage by <.01%.
The diff coverage is 0%.

Impacted Files Coverage Δ
cmd/frontend/graphqlbackend/search_alert.go 6.97% <0%> (ø) ⬆️
pkg/conf/computed.go 20% <0%> (ø) ⬆️
cmd/frontend/db/external_services.go 0% <0%> (ø) ⬆️

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants