Skip to content
This repository was archived by the owner on Oct 11, 2023. It is now read-only.

Conversation

@joao-paulo-parity
Copy link
Contributor

@joao-paulo-parity joao-paulo-parity commented Mar 9, 2021

fixes #219

Takes the suggestion from #219 (comment) to trim down to the used variants only. It's quite likely more code can be removed throughout the project, but that's part of another issue (#243).

Initially, the approach was parsing the field as String and then matching them through an interface (#245); that approach was removed in favor of another suggestion (#244 (comment)).

@joao-paulo-parity joao-paulo-parity self-assigned this Mar 9, 2021
@joao-paulo-parity joao-paulo-parity marked this pull request as draft March 9, 2021 16:05
@joao-paulo-parity joao-paulo-parity marked this pull request as ready for review March 9, 2021 16:13
@joao-paulo-parity joao-paulo-parity marked this pull request as draft March 10, 2021 09:13
joao-paulo-parity added a commit that referenced this pull request Mar 10, 2021
use serde(other) to avoid parsing errors on unused variants

suggested in #244 (comment)
@joao-paulo-parity joao-paulo-parity marked this pull request as ready for review March 10, 2021 09:26
Copy link

@ordian ordian left a comment

Choose a reason for hiding this comment

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

It's strange that we only use two event types and two fields, but if the code compiles, then it's fine.

In the future we might also want to keep track of Closed, Merged and Reopened and probably other variants.

use serde(other) to avoid parsing errors on unused variants

suggested in #244 (comment)
@joao-paulo-parity joao-paulo-parity merged commit 0ebe644 into master Mar 10, 2021
@joao-paulo-parity joao-paulo-parity deleted the update_variants branch March 10, 2021 15:58
@joao-paulo-parity joao-paulo-parity added this to the 0.6 milestone Mar 29, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

'bot merge' errors

4 participants