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

[7/7] Remove array_get #1385

Merged

Conversation

jchaffraix
Copy link
Collaborator

@jchaffraix jchaffraix commented Dec 8, 2024

The function cannot be properly typed as it
is and fixing this would take a while, removing
it is shorter and better.

The change removes usage in the last remaining
files. It also removes 2 tests for the function
that were converted instead of removed.

TESTS=Tested site/author searching and some
project updates.

Sandbox at https://www.pgdp.org/~cpeel/c.branch/julien_remove_array_get_7/

The function cannot be properly typed as it
is and fixing this would take a while, removing
it is shorter and better.

The change removes usage in the last remaining
files. It also removes 2 tests for the function
that were converted instead of removed.

TESTS=Tested site/author searching and some
project updates.
@jchaffraix jchaffraix marked this pull request as ready for review December 8, 2024 01:47
Copy link
Member

@cpeel cpeel left a comment

Choose a reason for hiding this comment

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

Thank you so much for this code clean-up and working through the multiple PRs to get it tested and in!

@cpeel cpeel requested review from chrismiceli, 70ray and srjfoo December 8, 2024 02:06
@cpeel
Copy link
Member

cpeel commented Dec 8, 2024

I've added a commit to this branch that updates the SETUP/CHANGELOG.md about this function being deprecated.

@cpeel cpeel merged commit 9fba0b2 into DistributedProofreaders:master Dec 9, 2024
12 checks passed
@jchaffraix jchaffraix deleted the julien_remove_array_get_7 branch December 17, 2024 07:17
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.

5 participants