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

Updated Google reCaptcha V1 to Google reCaptcha V2 #47

Merged
merged 10 commits into from Mar 2, 2017
Merged

Updated Google reCaptcha V1 to Google reCaptcha V2 #47

merged 10 commits into from Mar 2, 2017

Conversation

douglasparker
Copy link

I've updated Google reCaptcha V1 to Google reCaptcha V2. I also updated recaptchalib.php to the latest version: https://github.com/google/recaptcha/blob/1.0.0/php/recaptchalib.php

As you can see, I kinda messed up and ended up with a ton of commits. Sorry about that.

Copy link
Member

@MishimaHaruna MishimaHaruna left a comment

Choose a reason for hiding this comment

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

Don't worry about the extra commits, I can squash them while merging the pull request.

$_POST['recaptcha_response_field']);

if (!$resp->is_valid) {
$response;
Copy link
Member

Choose a reason for hiding this comment

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

Does this line have a purpose?

Copy link
Author

Choose a reason for hiding this comment

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

It's intended to be $response = false; basically.

I suppose I could change it to $response = $_POST["g-recaptcha-response"]; which is the client side check with the form and then change if($_POST["g-recaptcha-response"]) to if($response).

Copy link
Member

Choose a reason for hiding this comment

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

Please do, it makes the purpose much more clear

$_POST['recaptcha_response_field']);

if (!$resp->is_valid) {
$response;
Copy link
Member

Choose a reason for hiding this comment

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

Same as above, does this have a purpose, or is it intended to be $response = false; or something similar?

Copy link
Author

Choose a reason for hiding this comment

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

This is the same as above but for the login captcha.

Lavenblade added 2 commits February 22, 2017 02:37
Merged branch master into master
Updated Google reCaptcha V1 to Google reCaptcha V2


Updated Google reCaptcha V1 to Google reCaptcha V2


Updated Google reCaptcha V1 to Google reCaptcha V2
Updated Google reCaptcha V1 to Google reCaptcha V2
Updated ReCaptchaTheme for Google reCaptcha V2
@douglasparker
Copy link
Author

douglasparker commented Feb 22, 2017

Alright, done with the changes.

Any advice on how to avoid the commit spam? I tried squashing them using my git client but they still show up on github.

Also, I don't know why it's adding and readding lines to commits. It makes reviewing changes a real pain. =/

Edit: Fixed the newline issue. This shouldn't be a problem anymore. Had to do with a package in atom.io

@MishimaHaruna
Copy link
Member

To squash, I do the following. It assumes that you have two remotes: upstream pointing to HerculesWS/FluxCP and origin pointing to Lavenblade/FluxCP, and that you are currently on the branch you opened the pull request from (so change as needed for your setup).

git fetch --all                 # Make sure you're up to date
git rebase -i upstream/master   # Rebase your branch on top of master in interactive mode
# Now change all the 'pick' into 'squash' for all the commits you want to squash;
#    leave 'pick for the first commit you want to keep, then save and quit the editor
git push --force                # Push your changes, overwriting the current content of
                                # your branch (git needs this to know that you know what
                                # you're doing, since a rebase operation is potentially
                                # dangerous, since it rewrites history, deleting and
                                # replacing existing commits from that branch)

But don't worry about it now, I can just squash them from the GitHub UI while merging.

For the newline issue: yeah, it's mostly the FluxCP codebase's fault though. The original author had their text editor severely misconfigured, and was leaving a mess of trailing whitespace on empty lines.

@douglasparker
Copy link
Author

Awesome, thanks for the tip!

Are there any other changes that need made? Would you like me to host a copy of FluxCP with this working before pulling?

@FlippAcademy FlippAcademy merged commit c83e979 into HerculesWS:master Mar 2, 2017
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.

None yet

3 participants