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

ref(processor): Remove the processor state #4416

Merged
merged 3 commits into from
Dec 20, 2024

Conversation

iambriccardo
Copy link
Member

@iambriccardo iambriccardo commented Dec 20, 2024

This PR removes the processor state. The state object is what prevented us from taking a stab at refactoring processing groups in Relay.

In upcoming PRs, the new processor refactor will take place.

#skip-changelog


/// The partial result of the envelope processing.
#[derive(Debug)]
enum ProcessingPartialResult {
Copy link
Member Author

Choose a reason for hiding this comment

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

Created this partial result to be more clear about the fact that this result is not the full one because the envelope is added afterwards.

Copy link
Member

Choose a reason for hiding this comment

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

Can we have ProcessingExtractedMetrics on the ProcessingResult instead? That would make it simpler to understand IMO, with a PartialResult type the reader has to follow one additional hop.

@iambriccardo iambriccardo marked this pull request as ready for review December 20, 2024 09:19
@iambriccardo iambriccardo requested a review from a team as a code owner December 20, 2024 09:19

/// The partial result of the envelope processing.
#[derive(Debug)]
enum ProcessingPartialResult {
Copy link
Member

Choose a reason for hiding this comment

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

Can we have ProcessingExtractedMetrics on the ProcessingResult instead? That would make it simpler to understand IMO, with a PartialResult type the reader has to follow one additional hop.

@iambriccardo
Copy link
Member Author

@jjbayer yes, I could do that. The reason for my partial result struct is that I liked the NoMetrics and WithMetrics enum variants stylistically. In addition, I liked the idea of having a return type that incapsulates data, rather than exposing the extracted metrics directly.

@iambriccardo iambriccardo merged commit 7993960 into master Dec 20, 2024
23 checks passed
@iambriccardo iambriccardo deleted the riccardo/ref/remove-state-field-10 branch December 20, 2024 14:51
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.

2 participants