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

Fix STOCKINFO tag generation for investment files #255

Merged
merged 5 commits into from
Jan 2, 2024

Conversation

jaik03
Copy link
Contributor

@jaik03 jaik03 commented Jan 2, 2024

OFX STOCKINFO tags need to be closed and repeated for each individual security. Leaving the tag off leads to unexpected behavior in GNUCash (and potentially other software).

OFX STOCKINFO tag should be repeated for each individual security.
OFX STOCKINFO tag should be closed for each individual security
@kedder
Copy link
Owner

kedder commented Jan 2, 2024

Yeah, according to [this] you seem to be right.

image

Meanwhile, it looks like ofxstatement build got completely broken. I'll fix it in a separate PR.

@jaik03
Copy link
Contributor Author

jaik03 commented Jan 2, 2024

Thanks. I'm developing a plugin to import JSON brokerage exports from Schwab, and this was giving me fits testing the import into GNUCash. I'll have another couple minor PRs as I go.

@kedder
Copy link
Owner

kedder commented Jan 2, 2024

Could you please rebase this branch on master (to fix the build)?

@jaik03
Copy link
Contributor Author

jaik03 commented Jan 2, 2024

Could you please rebase this branch on master (to fix the build)?

Done!

Add STOCKINFO tags to start and end of every individual security.
Delete extraneous empty line so that Black won't throw an error.
@coveralls
Copy link

Coverage Status

coverage: 95.724%. remained the same
when pulling 6fa37fe on jaik03:Move-STOCKINFO
into 4c4a517 on kedder:master.

@jaik03 jaik03 marked this pull request as ready for review January 2, 2024 19:24
Copy link
Owner

@kedder kedder left a comment

Choose a reason for hiding this comment

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

LGTM

@kedder kedder merged commit 6916e58 into kedder:master Jan 2, 2024
6 checks passed
@jaik03 jaik03 deleted the Move-STOCKINFO branch January 2, 2024 19:26
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.

3 participants