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

Use serde(skip) instead of skip_serializing, add placeholder values #3362

Merged
merged 4 commits into from Jul 3, 2023

Conversation

Nutomic
Copy link
Member

@Nutomic Nutomic commented Jun 26, 2023

The latter breaks lemmy_crawler as the field is not included in the Lemmy API, but is required when attempting to parse API responses. Should only use serde(skip) to avoid this problem. I also had to add placeholder values so that deserialization works even though these mandatory fields arent being sent by Lemmy.

@Nutomic Nutomic requested a review from dessalines as a code owner June 26, 2023 14:44
@Nutomic Nutomic marked this pull request as draft June 26, 2023 14:46
@Nutomic Nutomic changed the title Use serde(skip) instead of skip_serializing Use serde(skip) instead of skip_serializing, add placeholder values Jun 26, 2023
@Nutomic Nutomic marked this pull request as ready for review June 26, 2023 20:19
@Nutomic Nutomic mentioned this pull request Jun 27, 2023
4 tasks
@bqv
Copy link

bqv commented Jun 27, 2023

Do we have api stability tests?

The latter breaks lemmy_crawler as the field is not included in
the Lemmy API, but is required when attempting to parse API responses.
Should only use serde(skip) to avoid this problem
@Nutomic
Copy link
Member Author

Nutomic commented Jun 28, 2023

@bqv Not really, but the code for lemmy-js-client is auto-generated from Lemmy source, so you will see any changes there in the git log.

@Nutomic
Copy link
Member Author

Nutomic commented Jun 30, 2023

Dont know why Github closed this.

@Nutomic Nutomic reopened this Jun 30, 2023
@Nutomic Nutomic merged commit cb91eed into main Jul 3, 2023
1 of 2 checks passed
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.

None yet

2 participants