Skip to content
This repository has been archived by the owner on Jun 10, 2022. It is now read-only.

Merge upstream 1.21.0 into our fork #9

Open
wants to merge 188 commits into
base: master
Choose a base branch
from
Open

Conversation

tilo
Copy link

@tilo tilo commented Feb 1, 2021

merging upstream/master @1.20.0 into our fork, so we can get rid of our private fork of this gem.

this also includes the changes which were pushed upstream to use HashWithIndifferentAccess
JsonApiClient#386
and
JsonApiClient#389

mordaroso and others added 30 commits March 20, 2018 10:22
…ized-attribs

Exclude path params from attributes hash
It is very happy if we can write `Model.page(params[:page])` like Kaminari.
…-nil

allow nil for page number parameter
Fix custom type comment to use TypeFactory
…-to-fix-empty-array-param-issue

JsonApiClient#278 updating faraday to fix issue when param value is an empty array
…eate-and-update-resource

allow to send fields and/or includes on create/update resource
…arning

Use mocha/minitest instead of mocha/mini_test on test_helper
…r-new-custom-connection-arguments

Update arguments for custom connections run method in README
fixing readme to update test and coverage icon targets
it will fix behavior when relationships and links are passed as symbol keys
…-pagination-override-example

JsonApiClient#291 import README - pagination override examples
…w-symbolized-relationship-key

JsonApiClient#290 symbolize params keys on model initialize
Copy link

@donovan-lampa donovan-lampa left a comment

Choose a reason for hiding this comment

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

Let's do it.

I see up above the concerns about testing and rollout, do we have a plan for how we want to go about rolling this out or testing it? One app at a time in QA first? Is there meaningful traffic we can push to apps in QA to get good enough coverage to feel confident enough? Is our integration testing story better now (would RING cover this?)

Tilo Sloboda and others added 4 commits August 19, 2021 09:33
@tilo
Copy link
Author

tilo commented Sep 6, 2021

@donovan-lampa I'll update it once more with the recent updates to the upstream - so the HashWithIndifferentAccess is also included.

@tilo tilo changed the title Merge upstream 1.18.0 into our fork Merge upstream 1.19.0 into our fork Sep 6, 2021
@tilo
Copy link
Author

tilo commented Sep 6, 2021

@corsonknowles @ldrbrandon @dpep @donovan-lampa @openbl @dalyons

Please note that I created a new branch "upstream" in our fork which forks off of 3f36f74, which contans all the upstream changes since we created our fork with our own modifications.

This makes it easier to see our actual diff:

upstream...merge-upstream-1.18.0

@tilo
Copy link
Author

tilo commented Sep 6, 2021

I'll create more PRs for the upstream to bring our changes in, e.g. the bang methods, to make our diff even smaller

JsonApiClient#389

@tilo tilo changed the title Merge upstream 1.19.0 into our fork Merge upstream 1.20.0 into our fork Sep 8, 2021
@tilo
Copy link
Author

tilo commented Sep 8, 2021

@donovan-lampa @dpep @corsonknowles @ldrbrandon @openbl
upstream has merged my PR in, and i pulled the latest upstream 1.20.0 into this branch.. the new diff is tiny,
basically just additional tests we added

upstream...merge-upstream-1.18.0

@tilo
Copy link
Author

tilo commented Sep 8, 2021

@maciej-chime
Copy link

do we still need to be bumping this, or should we focus on getting the remaining chime-service (and libraries?) onto the public version of the gem?

@tilo
Copy link
Author

tilo commented Sep 28, 2021

at this point, we can just try to upgrade the dependent repos to the public version of json_api_client

https://github.com/search?q=org%3A1debit+1debit%2Fjson_api_client&type=code

@tilo tilo changed the title Merge upstream 1.20.0 into our fork Merge upstream 1.21.0 into our fork Mar 11, 2022
@tilo
Copy link
Author

tilo commented Mar 11, 2022

wonder if we could accomplish our diff by inheriting our own resource class to do those things instead of needing to hack their library

yes, totally agree - we should not have forked this repository in the first place

@corsonknowles
Copy link

Do you still want to merge this?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.