search

drdhaval2785 / github_issue_backup

a script to backup github issues
0 stars 0 forks source link

Jim suggestions #28

Closed drdhaval2785 closed 9 years ago

drdhaval2785 commented 9 years ago
  1. As far as I can tell, the html/images directory in each repository backup does NOT have all the embedded comment images. However, the html displays properly display the Github images.
  2. @drdhaval2785 I used all the introductory material you provided, which was good to get me up to speed. However, it took several tries to get the exact form of the above command. The readme might need a little more attention at this point.
  3. Also, I notice that you have authentication codes in the script. While these worked fine, I'm a bit concerned that you might be leaving yourself open to security issues -- Not sure if this is so, but thought it should be mentioned.
drdhaval2785 commented 9 years ago
  1. The issue was known to me. It was because Bash takes arrays to be separated by space. So $x in bash was 1 2 3 5 10.... When it was passed as an argument to PHP like php image_links.php $1 $yy $x, PHP took 1 to be $argv[3], 2 to be $argv[4] etc.. Therefore, PHP only took the images in the issue 1 i.e. $argv[3]. Now the code is corrected. Now before processing, I club all the arguments passed to PHP into $argv[3] and then fetch images. The same issue was there in substitute_images.php. Both have been corrected. So, now all images of all issues are fetched. Also, all images are suitably referenced to local copies in images folder rather than the web images.
  2. As the code was not stable as shown in point 1, readme was not updated. Now the readme is updated with the details about all arguments and also 10 working examples of various permutation and combinations.
  3. I thought about removing authentication codes. These are tool specific codes. If I remove the codes, I will have to provide for additional two arguments in the script like username password. If you can show me potential dangers (which I think must be there), I would do so. But, the codes are 50 letters words. Passing them in argument doesn't seem a viable option either.

So issue 1 and 2 resolved. 3 pending for @gasyoun and @funderburkjim to explore.