blasee / fetchR

Calculate Wind Fetch in R
4 stars 3 forks source link

Cleanup and other various changes #1

Closed sjp closed 10 years ago

sjp commented 10 years ago

Hi Blake -- another package another pull request.

Aside from adding in the usual Travis CI support, I've ensured that the fetchR now passes the package checker. In the process I've dropped the dependency on rgeos, as it didn't seem to be used at all.

I've also had a go had a code review. Mostly this involved the following:

These changes have not been tested by myself so I can't guarantee whether they work, but they are at least syntactically correct.

A few nit-picky thoughts as an aside:

Congrats on yet another package! Keep up the good work!

blasee commented 10 years ago

Thanks for the tips and reviews Simon!

fetchR (unlike Rome) was built in a day so there are a few concepts I am not too happy with either like having the plotting within the same calculation function - as you point out. I'll take these considerations on board and once again thanks heaps for your contributions!