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

Rename Request#response to Request#perform #1297

Merged
merged 1 commit into from
Jan 11, 2024
Merged

Conversation

st0012
Copy link
Member

@st0012 st0012 commented Jan 11, 2024

Motivation

As @vinistock pointed out here, using #response seems to imply that the request was already completed before it's called. In ruby-lsp's case, many requests are used in this pattern:

FooRequest.new(args).response

In this context, it feels like the computation is done in FooRequest#initialize and #response is just a getter, which is a bit misleading.

So this PR renames #response to #perform to make it more clear.

Implementation

Renames relevant interfaces and callers.

Automated Tests

Manual Tests

As @vinistock pointed out, using `#response` seems to imply that the
request was already completed before it's called. In ruby-lsp's case,
many requests are used in this pattern:

```rb
FooRequest.new(args).response
```

In this context, it feels like the computation is done in
`FooRequest#initialize` and `#response` is just a getter, which is a bit
misleading.

So this commit renames `#response` to `#perform` to make it more clear.
@st0012 st0012 added the chore Chore task label Jan 11, 2024
@st0012 st0012 requested a review from a team as a code owner January 11, 2024 16:58
@st0012 st0012 requested review from egiurleo and KaanOzkan January 11, 2024 16:58
Copy link
Member

@vinistock vinistock left a comment

Choose a reason for hiding this comment

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

Thanks!

@st0012 st0012 merged commit 6c1da44 into main Jan 11, 2024
30 of 36 checks passed
@st0012 st0012 deleted the rename-response-to-perform branch January 11, 2024 18:40
@st0012 st0012 removed the chore Chore task label Jan 11, 2024
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.

2 participants