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

Remove apply_transaction helper #5598

Merged
merged 1 commit into from
Jan 21, 2023

Conversation

pickfire
Copy link
Contributor

Not sure why there is this TODO, so I cleaned it up.

@CBenoit CBenoit added the S-waiting-on-review Status: Awaiting review from a maintainer. label Jan 20, 2023
Copy link
Member

@the-mikedavis the-mikedavis left a comment

Choose a reason for hiding this comment

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

Ah I forgot about this TODO: we removed the need for the apply_transaction helper a while ago but didn't want to complicate the diff. I think we're safe to remove it now 👍

There's one more reference of it in the integration test suite helpers to be removed

@pickfire
Copy link
Contributor Author

pickfire commented Jan 20, 2023

There's one more reference of it in the integration test suite helpers to be removed

I can't find it with rg apply_transaction.

@the-mikedavis
Copy link
Member

Ah actually looks like there's two of them

helix_view::apply_transaction(&transaction, doc, view);
and
helix_view::apply_transaction(&trans, doc, view);

@pickfire
Copy link
Contributor Author

Oh, looks like I accidentally added test/ to my git ignore, no wonder.

@the-mikedavis the-mikedavis merged commit a20a96a into helix-editor:master Jan 21, 2023
@the-mikedavis
Copy link
Member

Thanks for cleaning this up!

@pickfire pickfire deleted the apply-transaction branch January 23, 2023 02:57
kirawi pushed a commit to kirawi/helix that referenced this pull request Jan 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from a maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants