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 multi_json as dependency in favor of std-lib json #213

Closed
wants to merge 6 commits into from
Closed

Remove multi_json as dependency in favor of std-lib json #213

wants to merge 6 commits into from

Conversation

otherguy
Copy link
Contributor

@otherguy otherguy commented Mar 8, 2024

This PR is almost identical to #212 but for security reasons we want to maintain our own fork.

CI is also updated to include newer Ruby and Rails versions (currently failing on edge but that is expected).

@otherguy otherguy requested a review from byroot September 4, 2024 15:16
@byroot
Copy link
Member

byroot commented Sep 4, 2024

Looks fine, but lots of issues with CI. Let's get the MultiJson removal and CI fixes in different PRs.

@byroot byroot closed this in 536716a Sep 4, 2024
@byroot
Copy link
Member

byroot commented Sep 4, 2024

I merged the dependency removal.

However I tried to really update the CI matrix but it's a huge pain -_-.

@otherguy
Copy link
Contributor Author

otherguy commented Sep 4, 2024

@byroot thank you for resolving this with 536716a :)

As for the CI matrix, perhaps you can add 7.2 and make failures of edge just warnings?

@byroot
Copy link
Member

byroot commented Sep 4, 2024

The failure on edge is also on 7.2. I dug a bit as to what is going on, but I'm a bit tired.

@otherguy
Copy link
Contributor Author

@byroot can I help you get this released as a new version? What's missing?

@byroot
Copy link
Member

byroot commented Nov 25, 2024

The CI would need to be fixed, tests are no longer passing against Rails edge because of some change in the router.

I know the gem works fine, it's just a test helper that was reaching into Action Dispatch internals that need to be refactored/updated.

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