Skip to content

Switch wp_safe_redirect back to wp_redirect #67

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

Merged
merged 3 commits into from
May 31, 2021
Merged

Conversation

rmccue
Copy link
Member

@rmccue rmccue commented May 31, 2021

This redirect is intentionally open, so must use the regular redirect function.

This was inadvertently broken as part of #58.

This redirect is *intentionally* open, so must use the regular redirect function.
@rmccue rmccue requested a review from kadamwhite May 31, 2021 11:56
Copy link
Contributor

@kadamwhite kadamwhite left a comment

Choose a reason for hiding this comment

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

Code comment to prevent future issues, maybe?

@rmccue
Copy link
Member Author

rmccue commented May 31, 2021

I included the reasoning within the phpcs:ignore line; can add more if it's necessary?

@kadamwhite
Copy link
Contributor

Nah that's good, I didn't scroll far enough right and forgot it could be explained online!

@rmccue
Copy link
Member Author

rmccue commented May 31, 2021

Well, as it happens, I had the wrong sniff code anyway 😒

WPCS is additionally failing due to the (I guess new?) "Short array syntax is not allowed (Generic.Arrays.DisallowShortArraySyntax.Found)" error here, as well as "date() is affected by runtime timezone changes which can cause date/time to be incorrectly displayed. Use gmdate() instead. (WordPress.DateTime.RestrictedFunctions.date_date)".

Should I add these errors to the ignored rules? Should we have a separate PR for fixing them? The function change here has broken the entire OAuth system, so it's key that we get it fixed (i.e. the current head version just doesn't work).

@rmccue rmccue mentioned this pull request May 31, 2021
@rmccue
Copy link
Member Author

rmccue commented May 31, 2021

Should I add these errors to the ignored rules? Should we have a separate PR for fixing them?

Tackled in #68; I can merge that into this branch and phpcs should pass, if desired.

@kadamwhite
Copy link
Contributor

Going to proceed with merge, fix looks good and the branch management makes sense

@kadamwhite kadamwhite merged commit dedbd68 into master May 31, 2021
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