search

hoodie / icalendar-rs

📆 icalendar library, in Rust of course - for fun
Apache License 2.0
126 stars 34 forks source link

refactor: remove `done` and consuming builder #24

Closed ModProg closed 2 years ago

ModProg commented 3 years ago

Now uses a builder that takes self and returns Self. That way it does not require to call done before using the result.

Renamed add_parameter to parameter to match the other builder functions.

This improves usage of the API and cuts down on code in the implementation.

ModProg commented 3 years ago

I will experiment with refactoring the library a little in preparation for the parsing and fields of non string types.

If this ends up being a too big change for this library, I'd be fine with creating a separate crate of the fork.

hoodie commented 3 years ago

nice, thanks

hoodie commented 3 years ago

thanks

ModProg commented 3 years ago 
* [ ]  could you maker this a `feat:` in the commit message and add the conventional `# BREAKING CHANGE` label a la [conventional commits](https://www.conventionalcommits.org/en/v1.0.0/)

I will, I would like to experiment with some further refactorings before merging if that is fine for you.

hoodie commented 3 years ago

hmm. I'd rather keep changes separated. I'd merge this if you don't mind and then treat everything else separately

ModProg commented 3 years ago

Alright :+1:

hoodie commented 2 years ago

actually, this has the effect that you can't use a mutable object anymore. since the builder methods are always consuming the Event you cannot just hold one and change something about it. You'll always have to reassign to your variable. I think that's why I originally didn't do this.

ModProg commented 2 years ago

actually, this has the effect that you can't use a mutable object anymore. since the builder methods are always consuming the Event you cannot just hold one and change something about it. You'll always have to reassign to your variable. I think that's why I originally didn't do this.

That is correct.

As done was already consuming it's input by emptying it, I went this route.

Maybe an alternative would be having a separate set, or just keep it as it is.