search

FlominatorTM / wikiblame

http://wikipedia.ramselehof.de/wikiblame.php
GNU General Public License v3.0
54 stars 13 forks source link

Remove functions that are now unused #18

Closed vlakoff closed 6 years ago

vlakoff commented 6 years ago

A little code cleanup, as I mentioned on #17.

Also refs: https://github.com/FlominatorTM/wikiblame/commit/cc06b70fbf3f79da343552afe6d09c789f045f53, https://github.com/FlominatorTM/wikiblame/commit/c67427644fca626b13ce41583e570b39b60c92f1.

vlakoff commented 6 years ago

You might want to add back a set_time_limit into the get_history() function (a 5000-line history page is quite large).

edit: maybe also in get_all_versions()?

FlominatorTM commented 6 years ago

Some of the old tool on my hard disk used these shared functions, but I changed them now to comply.

You're probably right with set_time_limit(), but so far I didn't discover any problems in my other tools related to this.

vlakoff commented 6 years ago

I mean that this PR removes the set_time_limit that was executed in get_history(), through the old get_request(). So, to be on the safe side, you might want to add it back, directly in get_history().

And for consistency, you may also add it to get_all_versions() (though it wasn't executing set_time_limit since the switch to file_get_contents).

This way, the time limit would be reset on each revision and history requests, which seems complete and robust enough.

Finally, only you knows how the wikiblame server behaves, and the various network issues it encounters :)

vlakoff commented 6 years ago

By the way, the old get_request() function was adding an user agent ("script by de_user_Flominator"). You might want to add back an user agent to the curl_request() function (CURLOPT_USERAGENT in the options array), as it's usually considered good practice.

See also this question on webmasters.stackexchange.com and in particular the accepted answer.

FlominatorTM commented 6 years ago

Did that with https://github.com/FlominatorTM/wikipedia_shared_includes/commit/009b4f8592ca2fce9331dfa2d27ebec1fc637b37

btw: Apparently the google search above did not cover everything, because at least one get_request slipped (see https://github.com/FlominatorTM/wikipedia_wbw/commit/9d913308f57b0557ffe42fb3360cbdd7247c5734 )

vlakoff commented 6 years ago

I don't remember if I overlooked these matches at the time of writing (thinking it was a fork or whatever), or if Google wasn't indexing them. But it does index them currently (post_request, get_request), and I suppose there are no other leftovers.