search

TYPO3-Caretaker / caretaker_instance

TYPO3 extension caretaker_instance
GNU General Public License v2.0
14 stars 23 forks source link

Disk space usage #5

Closed tomasnorre closed 9 years ago

tomasnorre commented 11 years ago

I have created a Caretaker test for checking free diskspace at server. Please let me know what you think.

etobi commented 9 years ago

Hej, first of all: sorry to let you wait for the review of your PR for so long.

I like to not merge your change for several reasons:

  1. The PullRequests says "Disk space usage", however to commits contain two new services: the "Version Control" test and the "disk space" test. It would be easier and cleaner to review if you could split it up into two pull requests (/two branches and a PR for each of both).
  2. regarding the VersionControl: when we provide such service, we should support Git, too, as in the meantime this might be used in the most cases.
  3. regarding both new services: If i'm right from reading your commits, the version control aswell as the disk usage is only checked on the local system (the caretaker server). I think the user will expect to execute these tests on the Instance instead. Thus two new operations (for VersionControl and for the diskspace) need to be implemented. The TestService executes these remote operations on the remote instance, checks the returned values and decides if the test was okay or not.

I will close this pull request for now. If you feel like giving it a new try, just open a new PullRequest. We really appreciate any contribution. Even if it takes ages if one of us finds the time to review them. I feel very sorry about that.

regards tobias

tomasnorre commented 9 years ago

Hej ;)

It's been a while true, but no the less, feedback is given.

I'm not using caretaker anymore as I switched job, but @ulrikkold at Combine, which i referred to the other day when you search for sponsor is still using it to my knowledge at least.

These tests are really old, and the last time i touched Caretaker, i understand you reason for rejections, if i find the time, i'll rewrite them, but perhaps is doesn't make sense before caretaker is refactored, so i rather spend some time refactoring Caretaker than fixing this pull request.

So let me know if I somehow can help you with the caretaker refactoring, still considering it for private projects, but right now i'm not using it.

Cheers Tomas