-
Notifications
You must be signed in to change notification settings - Fork 699
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 Itp
from LoginProtection
#1604
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like we are missing the changelog notes / upgrading notes. Can you explain in both docs why this was removed and how people who might still be using it can patch their app before we delete Itp?
If any of your controllers are dependant on methods from `Itp` then you can include `ShopifyApp::Itp` directly. | ||
You may notice a deprecation notice saying, `Itp will be removed in an upcoming version`. | ||
This is because we intend on removing `Itp` completely in `v22.0.0`, but this will work in the meantime. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hope this is the right place?
I was also thinking of adding the names of the methods like set_test_cookie
, set_top_level_oauth_cookie
, clear_top_level_oauth_cookie
, ....
So it would be easy to search for that method being removed in the upgrading doc. How do you feel about that?
ooops sorry about missing this, I've been so scatterbrained lately |
Co-authored-by: Nelson <[email protected]>
Co-authored-by: Nelson <[email protected]>
What this PR does
Itp
's deprecation logs were getting sent when loadingLoginProtection
.Checklist
Before submitting the PR, please consider if any of the following are needed:
CHANGELOG.md
if the changes would impact usersREADME.md
, if appropriate./docs
, if necessary