ashwin47 / Fb-badge

Facebook profile picture overlay/badge creater (unmaintained)
37 stars 41 forks source link

Fixed various problems with repo #1

Closed rhnvrm closed 9 years ago

rhnvrm commented 9 years ago

Hi

I found some problems with the repo and have fixed them. Here is the summary of my changes:

  1. Removed your credentials from the history by using filter-branch
  2. Added the missing facebook_start.php which you probably removed yourself earlier but broke the project for cloners. So have added a cred.php file in which you can store your credentials and added a note in the readme. It is safely put in gitignore and will not be pushed.
  3. You had added files to gitignore but it had not been applied so had to fix untracked files.

The changes hopefully should be safe to merge.

Regards Rohan

ashwin47 commented 9 years ago

I have already run filter-branch. Its in effect right?!

rhnvrm commented 9 years ago

Somewhat, it wont appear directly in your repo but someone has put that tree's link on reddit and hundreds have probably seen it. I have run filter-branch so as to mistakenly not put your credentials again in my fork. This was the link of that tree posted on reddit: https://github.com/ashwin47/Net-Neutral/blob/f53d7c432920d36f2d342c7c7673de4ad0df887d/facebook_start.php

It would be better that you just reset your secret key from fb developer dashboard. It's true that there is probably no way to abuse it (because Facebook always checks origin URL, too) however you should not share it...

The main thing in my pull request is the way you handle your credentials. You have removed the file facebook_start.php which broke the project for anyone who wants to contribute to the project by cloning it/pulling it. So I have added a new file called cred.php in which you can store those credentials and not push them to the repo(added to .gitignore). You can see the git diff here: https://github.com/ashwin47/Net-Neutral/pull/1/files

If you want to test this, clone https://github.com/rhnvrm/Net-Neutral and run a php -S localhost:8080 in the directory. Otherwise, I have already tested this on my system and hope this helps your project.

Regards Rohan

ashwin47 commented 9 years ago

Isn't it more easier to rename _facebookstart.php to cred.php ?

rhnvrm commented 9 years ago

But cred.php won't be pushed to the repo... and you certainly need some of the code currently inside facebook_start.php to run the project locally.