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

Current Serializer is hard to maintain #175

Closed
torkleyy opened this issue Jun 6, 2019 · 4 comments
Closed

Current Serializer is hard to maintain #175

torkleyy opened this issue Jun 6, 2019 · 4 comments

Comments

@torkleyy
Copy link
Contributor

torkleyy commented Jun 6, 2019

The problem

The current serializer provides all kind of config parameters, but as it is now, it became hard to maintain.
Many things are special cased, and code is duplicated across tuples / enums / sequences / maps.

Additionally, we're lacking tests for pretty serialization.

How to fix this

We need to build an abstraction for comma separated values, which removes the duplication we have now.

Once that's done, special cases should be easier to identify.

bors bot added a commit that referenced this issue Apr 27, 2020
206: feat: port `ser::Serializer` to `io::Write` r=kvark a=Plecra

Makes the `Serializer` struct generic over `io::Write`, allowing it to be used with more types (and less allocation). As a side-effect, the trailing commas are also removed from the output.

This was a quick implementation, written in response to compiler messages, so it could probably do with some cleaning up. It might be a good time to approach #175 

Closes #202 

## TODO

- [ ] Update docs


Co-authored-by: Marli Frost <marli@frost.red>
bors bot added a commit that referenced this issue May 4, 2020
206: Port `ser::Serializer` to `io::Write` r=kvark a=Plecra

Makes the `Serializer` struct generic over `io::Write`, allowing it to be used with more types (and less allocation). As a side-effect, the trailing commas are also removed from the output.

This was a quick implementation, written in response to compiler messages, so it could probably do with some cleaning up. It might be a good time to approach #175 

Closes #202 

## TODO

- [ ] Update docs


Co-authored-by: Marli Frost <marli@frost.red>
@github-actions
Copy link

Issue has had no activity in the last 180 days and is going to be closed in 7 days if no further activity occurs

@github-actions github-actions bot added the stale label Nov 18, 2021
@kvark kvark reopened this Dec 3, 2021
@kvark kvark reopened this Dec 17, 2021
@torkleyy
Copy link
Contributor Author

This is still a problem, although we have more tests now thanks to @MomoLangenstein which should make refactoring easier.

@torkleyy torkleyy reopened this Dec 25, 2021
@github-actions github-actions bot closed this as completed Jan 2, 2022
@kvark
Copy link
Collaborator

kvark commented Jan 3, 2022

weird that GH actions bot closes this, even thought there is a comment?

@kvark kvark reopened this Jan 3, 2022
@github-actions github-actions bot removed the stale label Jan 4, 2022
@juntyr
Copy link
Member

juntyr commented Sep 3, 2023

I think that #206 fixed the code duplication issue and #488 finally pushed the serialising code to 100% code coverage

@juntyr juntyr closed this as completed Sep 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants