chusopr / casimir

Yet Another URL Shortener
http://casim.ir/
MIT License
18 stars 11 forks source link

mysqli php 5.5 deprecated errors #14

Closed neofutur closed 9 years ago

neofutur commented 9 years ago

hi

I fixed it in : https://github.com/neofutur/gwgd/commit/f08d74a3e45a520a34e39845ef987dd5f255329e converted all the mysql to mysqli functions.

but couldnt find a way to send a pull request for just that commit.

will remove errors like :

PHP Deprecated:  mysql_connect(): The mysql extension is deprecated and will be removed in the future: use mysqli or PDO instead in html/inc/Casimir.php
chusopr commented 9 years ago

I move this comment I made in commit instead of here:

Yes, my site log is flooded with some warnings too, I'm glad that finally someone was diligent enough to fix it ;-) Mysqli is supported since PHP 5.0 and MySQL 4.1.3 and is the recommended choice [1], so I think that it's a safe move. Anyway, I'd like to hear @nhoizey about this. I will apply this patch to my sites to test it.

Also, your last hunk fixes a different issue. Even though this fix looks fine for me too, I don't agree with mixing different fixes in a single commit. But this is just my opinion.

About submitting pull request, you can review GitHub guide: https://help.github.com/articles/using-pull-requests/ I usually make a separate branch for every issue that I want to fix and then send a pull request for that branch when the issue is fixed, so I can make more commits in my master without affecting that pull request.

[1] http://php.net/manual/en/mysqli.overview.php

chusopr commented 9 years ago

I think that you can send a pull request for this commit with something like this (not tested):

# Create a branch for that commit
git branch casimir-mysqli f08d74a
# Push branch to GitHub
git push origin casimir-mysqli
# Send pull request
curl --user neofutur --request POST \
--data '{"issue":"14", "head":"neofutur:casimir-mysqli", "base":"master"}' \
https://api.github.com/repos/nhoizey/casimir/pulls
nhoizey commented 9 years ago

@neofutur you indeed have to create a branch from that commit and make a pull request

chusopr commented 9 years ago

No news from @neofutur in two weeks.

I'm committing my own fix for this issues. The reasons to discard the provided one are the following:

Thanks to @neofutur for pointing the need to fix this.

neofutur commented 9 years ago

sorry for no news,i didnt receive any emails about this issue and i dont check github very often. yes our forks are already very different, np for re implementing it in yours, just wanted to warn you about this mysqli issue just in case if you re interested I just added a few basic antispam features, based on the spam flood i could find in my gw.gd : https://github.com/neofutur/gwgd/commit/ac593212fd55fd0086842525fffd2cbdf54247bb https://github.com/neofutur/gwgd/commit/75ff1e57edcf6e3403f4d02699049dfb62b32eb7

I ll try to learn more of git , but creating a new branch for every commit seems too much seen from here ;(

chusopr commented 9 years ago

Thanks, I will take a look. Also note that CAPTCHA support has been added using the new reCAPTCHA that is quite user-friendly (just a check box in most cases). https://github.com/chusopr/casimir/commit/67b1673a84dd8beb3b066cc4d9dbc9fc8c9eea8c