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

cmsis_update::update() #132

Open
Yatekii opened this issue Oct 31, 2019 · 1 comment
Open

cmsis_update::update() #132

Yatekii opened this issue Oct 31, 2019 · 1 comment

Comments

@Yatekii
Copy link
Contributor

Yatekii commented Oct 31, 2019

Hi!

Is there a reason that cmsis_update::install() for example requires some type with a trait implemented to be passed into it just to configure it and get some progress update?
Why can't this be fixed? Is it because of the python interface? Even then I think this can be implemented with a less complicated interface which I'd highly prefer.

Also, is there a reason you force users to use slog or did you just not care enough? Because I don't want to use slog but I now have to include the dependency anyways. And exactly for this scenario there is https://docs.rs/log/0.4.8/log/ which provides a nice and unified frontend for maaaany different backends (slogbeing one of them).

I know this crate was maybe not written with intent to be reused as a lib, so don't take my criticism too harsh, I just wanted to ask and would make those improvements in a PR if that's ok =)

Best,
Yatekii

@Yatekii Yatekii changed the title cmsis_update::update()` cmsis_update::update() Oct 31, 2019
@theotherjimmy
Copy link
Collaborator

some type with a trait implemented to be passed into it just to configure it and get some progress update?

There is probably a better way to do this. I'm open to suggestions and PRs for simpler interfaces. Originally it was to allow the python interface to handle the messages differently than the CLI.

Is there a reason you force users to use slog or did you just not care enough?

I don't think slog is needed at this point. I must have had a reason for adding it, but that reason is probably no longer applicable. I think it would be some work to pull out the usage of slog.

I just wanted to ask and would make those improvements in a PR if that's ok =)

I would love to see these improvements. Feel free to submit PRs.

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

No branches or pull requests

2 participants