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

refactor(probeservices): use better naming #1628

Merged
merged 8 commits into from
Jun 25, 2024
Merged

refactor(probeservices): use better naming #1628

merged 8 commits into from
Jun 25, 2024

Conversation

bassosimone
Copy link
Contributor

Calling the endpoint base URL (e.g., https://www.example.com/) "endpoint" could cause confusion because usually an API endpoint is something like https://www.example.com/api/v1/check-in. To obviate this semantic issue, this diff attempts to avoid calling base URLs as endpoint throughout the tree.

I am going to account this work as part of ooni/backend#754 because this is one of the possibly issues on which we can account this work. The original conversation with @ainghazal, which prodded me to make these changes, was related to his feedback after implementing #1625.

// ErrUnsupportedEndpoint indicates that we don't support this endpoint type.
ErrUnsupportedEndpoint = errors.New("probe services: unsupported endpoint type")
// ErrUnsupportedServiceType indicates that we don't support this service type.
ErrUnsupportedServiceType = errors.New("probe services: unsupported service type")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We generally do not change error strings. However, this error string is for a very uncommon case, because the service type should always be supported and so it's fine to modify it for consistency.

@@ -384,7 +386,7 @@ func TestOpenReportNewClientFailure(t *testing.T) {
Type: "antani",
}
err = exp.OpenReportContext(context.Background())
if err.Error() != "probe services: unsupported endpoint type" {
if !errors.Is(err, probeservices.ErrUnsupportedServiceType) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I upgraded this line to use errors.Is which is the idiomatic way of checking after errors.Is was added to Go.

@bassosimone bassosimone marked this pull request as ready for review June 25, 2024 13:56
@bassosimone bassosimone marked this pull request as draft June 25, 2024 14:18
@bassosimone bassosimone changed the title refactor(httpclientx): use better naming refactor(probeservices): use better naming Jun 25, 2024
@bassosimone bassosimone marked this pull request as ready for review June 25, 2024 14:27
Copy link
Contributor

@ainghazal ainghazal left a comment

Choose a reason for hiding this comment

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

lgtm

@bassosimone bassosimone merged commit 65df439 into master Jun 25, 2024
19 checks passed
@bassosimone bassosimone deleted the renaming branch June 25, 2024 14:49
@bassosimone bassosimone added the 2024-06-richer-input Tracking 2024-06 richer input work label Jul 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2024-06-richer-input Tracking 2024-06 richer input work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants