Cleanup old code - Githubissues

KentonWhite / ProjectTemplate

A template utility for R projects that provides a skeletal project.

http://projecttemplate.net

GNU General Public License v3.0

623 stars 159 forks source link

Cleanup old code #240

Open Hugovdberg opened 6 years ago

Hugovdberg commented 6 years ago

While documenting all functions I discovered a few functions that seem to be unused anywhere. They are not called from other functions and are not exported into the namespace. I'm listing candidates for removal here, please comment to substantiate why we should keep them:

[ ] cache.name in R/cache.name.R
[ ] .cache.status in R/cache.R
[ ] .unload.project in R/load.project.R (This function is only called from two tests where they seem to test completely unrelated stuff. Those function calls should be removed anyway, if clearing memory is necessary clear should be preferred)

If you think more functions can be removed also please comment.

These functions should be marked deprecated by use of the .Deprecated function, so we later can mark them .Defunct and eventually remove them altogether.

KentonWhite commented 6 years ago

Agree to removing cache.name and .cache.status. Deprecating first sound like a good approach.

unload.project is used when testing the migration code. I'm not yet sure if we want to remove the migration testing. It served a purpose in the past (when we had some pretty major changes and needed to ensure that all migrations were done properly) and could be useful once again!

rsangole commented 6 years ago

Not a function, but there's a 4year+ old TODO document which seems out of date.

[x] TODO.markdown at ProjectTemplate/TODO.markdown

KentonWhite commented 6 years ago

I think it is safe to remove the TODO.markdown file since we are tracking issues now on GitHub!

rsangole commented 6 years ago

I had this in #210 , but I think it's more appropriate here.

When I was working on #234 , I found that create.project() has two calls to .stopifproject which seem redundant.

https://github.com/KentonWhite/ProjectTemplate/blob/8072e518c3b286aca862104ad36f2ca6bc0cfc62/R/create.project.R#L49-L54

The 1st call works fine and stops correctly. If the 1st call is commented out, the 2nd call does not work correctly and creates a project within a project.

KentonWhite commented 6 years ago

Nice spot @rsangole. Lets move the fix here as you suggest.

Hugovdberg commented 6 years ago

Hmm I'm actually thinking that is just a separate bug, I was working on a fix but haven't created the PR yet. It really are two separate checks which both are useful and can't be merged. They just check the wrong directory.

KentonWhite commented 6 years ago

ok I'll reopen #210