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

[Merged by Bors] - Relax mutability requirements of FluvioAdmin client #1179

Closed
wants to merge 2 commits into from

Conversation

nicholastmosher
Copy link
Contributor

Closes #1178. Will help to prevent bugs like infinyon/fluvio-client-wasm#42

  • Changes &mut self to &self for create, delete, and list methods on FluvioAdmin.

Copy link
Contributor

@sehz sehz left a comment

Choose a reason for hiding this comment

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

Is this backwards compatible or requires major bump?

@nicholastmosher
Copy link
Contributor Author

I believe this is totally backwards compatible. Passing a &mut T where a &T is expected is totally valid. The only thing that will change for callers is they may get a warning if they do the following:

let mut admin = fluvio.admin().await; // Warning: `admin` does not need to be mutable
admin.list::<TopicSpec>(vec![]).await?;

@nicholastmosher
Copy link
Contributor Author

nicholastmosher commented Jun 18, 2021

Let me see how hard it's going to be to fix clippy (new lints from 1.53)

@sehz
Copy link
Contributor

sehz commented Jun 18, 2021

Can you put 1.53 clippy fix as separate PR?

Copy link
Contributor

@sehz sehz left a comment

Choose a reason for hiding this comment

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

LGTM. Nice improvement!

@nicholastmosher
Copy link
Contributor Author

Should pass now that clippy fixes are merged.

bors r+

bors bot pushed a commit that referenced this pull request Jun 18, 2021
Closes #1178. Will help to prevent bugs like infinyon/fluvio-client-wasm#42

- Changes `&mut self` to `&self` for `create`, `delete`, and `list` methods on `FluvioAdmin`.
@bors
Copy link

bors bot commented Jun 18, 2021

Timed out.

@nicholastmosher
Copy link
Contributor Author

Dang, I think something wonky happened with the macos job and caused it to take a really long time. Let's see if that was a fluke. The workflow was like 2 minutes from being done when Bors timed out 😭

bors r+

bors bot pushed a commit that referenced this pull request Jun 18, 2021
Closes #1178. Will help to prevent bugs like infinyon/fluvio-client-wasm#42

- Changes `&mut self` to `&self` for `create`, `delete`, and `list` methods on `FluvioAdmin`.
@bors
Copy link

bors bot commented Jun 18, 2021

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title Relax mutability requirements of FluvioAdmin client [Merged by Bors] - Relax mutability requirements of FluvioAdmin client Jun 18, 2021
@bors bors bot closed this Jun 18, 2021
@nicholastmosher nicholastmosher deleted the admin-tweaks branch June 21, 2021 16:29
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.

Relax mutability requirements for Fluvio Admin client
3 participants