Skip to content

Renaming apply #570

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

Merged
merged 1 commit into from
Jun 25, 2018
Merged

Renaming apply #570

merged 1 commit into from
Jun 25, 2018

Conversation

zoewangg
Copy link
Contributor

@zoewangg zoewangg commented Jun 22, 2018

Renaming apply() to make it more kotlin friendly.

  • SdkBuilder.apply -> SdkBuilder.applyMutation
  • ResponseTransformer.apply -> ResponseTransformer.transform

@kiiadi
Copy link
Contributor

kiiadi commented Jun 22, 2018

I get that applyMutator describes exactly what you're doing - you're applying a mutator to the builder - but I'd prefer to name it based the desired outcome rather than how you're doing it. You want to update the builder or mutate it. I think they're better options - alternatively if you want to keep the apply verb in the method name for discoverability purposes then applyMutation or applyChange.

@zoewangg zoewangg changed the title rename SdkBuilder.apply to SdkBuilder.applyMutator Renaming apply Jun 22, 2018
@zoewangg zoewangg requested review from kiiadi and shorea June 25, 2018 19:23
rename ResponseTransform.apply() to ResponseTransform.transform()
@zoewangg zoewangg force-pushed the zoewang-renameApply branch from e71a13b to 1a65053 Compare June 25, 2018 20:52
@zoewangg zoewangg merged commit f20161b into master Jun 25, 2018
@zoewangg zoewangg deleted the zoewang-renameApply branch June 25, 2018 21:10
aws-sdk-java-automation added a commit that referenced this pull request Jul 26, 2019
…b73b1f85

Pull request: release <- staging/0918b388-3368-49c7-984d-6e04b73b1f85
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