Skip to content

RevolutTransactions struct and TransactionAccumulator - #158

Merged
jczaja merged 3 commits into
mainfrom
sfraczek/separate-crypto
Sep 6, 2025
Merged

RevolutTransactions struct and TransactionAccumulator#158
jczaja merged 3 commits into
mainfrom
sfraczek/separate-crypto

Conversation

@sfraczek

@sfraczek sfraczek commented Jul 6, 2025

Copy link
Copy Markdown
Collaborator

Keep logic unchanged
Create TransactionAccumulator struct to group commonly used together 7 vectors
Create RevolutTransactions struct instead of a tuple to name returned fields from parse_revolut_transactions
add empty crypto_transactions field

Next, I will separate crypto into crypto_transactions

@jczaja please share your thoughts before I continue

Keep logic unchanged
Create RevolutTransactions struct instead of tuple
add empty crypto_transactions field
@sfraczek
sfraczek requested review from Copilot and jczaja July 6, 2025 08:23
@sfraczek sfraczek added the enhancement New feature or request label Jul 6, 2025

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the Revolut CSV parser by replacing the unstructured tuple return with a named RevolutTransactions struct, introduces a TransactionAccumulator to consolidate repeated vector parameters, and adds an empty crypto_transactions field for future use.

  • Grouped seven related vectors into TransactionAccumulator
  • Replaced the old tuple return of parse_revolut_transactions with the RevolutTransactions struct
  • Added an empty crypto_transactions field in both the public API and internal logic

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/lib.rs Updated call site to destructure the new RevolutTransactions struct instead of a tuple
src/csvparser.rs Introduced TransactionAccumulator and RevolutTransactions, changed parse_revolut_transactions signature and body to use those types
Comments suppressed due to low confidence (2)

src/csvparser.rs:19

  • [nitpick] Add a Rustdoc comment for TransactionAccumulator explaining its role in consolidating parsing state and supporting future extensions.
struct TransactionAccumulator {

src/csvparser.rs:449

  • [nitpick] Consider renaming the local variable ta to something more descriptive like accumulator or tx_accumulator to clarify its purpose.
    let mut ta = TransactionAccumulator::default();

Comment thread src/csvparser.rs Outdated
sfraczek and others added 2 commits July 6, 2025 10:54
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@jczaja
jczaja merged commit 57a2c26 into main Sep 6, 2025
5 checks passed
@jczaja

jczaja commented Sep 6, 2025

Copy link
Copy Markdown
Collaborator

good job!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants