-
Notifications
You must be signed in to change notification settings - Fork 96
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add FileBot Node plugin #215
Conversation
Hi @StephenBrown2 , Sorry again for not taking the time to look at your submission earlier. Thanks also for your latest commit db33ab4, as I was about to suggest to do just that. I tested the new version and it seems to work as intended:
The only doubt I have relates to the use of host networking (
without host networking:
Otherwise, thanks a lot for making the effort of providing all links and descriptions as we now try to, that is greatly appreciated! On another note, I personally wasn't familiar with Filebot before and didn't know what to expect just from reading the rock-on description. Maybe we could add a few more words to describe what the project does? From their website, we could take some of the following, for instance:
Thanks again for your contribution, and I apologize again for such a delay in getting back to you. |
Sure thing. I believe I must have copied the just networking from whatever other RockOn config I started with. I'll update that on Monday when I'm back at a computer, I had noticed the error too, but the functionality was unaffected. Also yes, it definitely needs a better description. I'll fix that too. |
Description updated, let me know if there's anything else that should be done. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It still works very well in my test, as it did in my previous tests. I only have a few minor changes that I can think of.
Hi @StephenBrown2 , First of all, let me apologize for the silence on this PR... things have been really busy on the main repository with almost all of our efforts spent on the relaunching the next Rockstor version and I'm afraid the rockon-registry repository has suffered from a lack of attention as a result. Thanks a lot for all your revisions and changes made to your original submission. It all looks good to me pending a few minor changes I highlighted in my review. There is one last thing I would recommend, however: I just now realized FileBot seems to require a license in order to function (after some time, I believe?) so we may want to indicate this briefly in the rock-on description. This way, users won't have an unexpected surprise if they go through the installation and setup process but fail to realize a license is required (that was my case, for instance). I understand this review might come a little too late, so my apologies again on this. |
Co-authored-by: FroggyFlox <30297881+FroggyFlox@users.noreply.github.com>
@FroggyFlox Much delay comes to all. I've applied your suggestions, and added a note regarding the License. Let me know if that's sufficient! |
Thanks a lot for taking care of it, @StephenBrown2 . |
@FroggyFlox I know this is super old but there has now been quite the effort put in so it would seem we are near enough on this one hopefully. Image was last updated around a month ago. Also re:
I'm pretty sure we have now, finally, gotten this short-coming in our back-end testing capability sorted now and so it should work for such a pull request as this one; where-as previously it was failing us. |
Thanks for the reminder, @phillxnet , and my deepest apologies for dropping the ball on that one. I've assigned myself to this PR so that I will be able to more easily have it on my to-do list. |
@FroggyFlox No worries. I was just doing some house-keeping and remembered upon seeing this that we fixed our back-end just recently. Thanks again for the help on that one by the way. |
@StephenBrown2 Just following up on the @FroggyFlox review. We do seem to have all requested changes now in place. I'd rather see the license in the description myself but it's there in the more-info for now and we can modify that at a later date, via a follow-up pr if there is the interest. Plus we link to the upstream Project https://www.filebot.net/ via the Rock-on name so I think we are up-front on that one all-in. And that page has the per-year/life-time License options just a little way down. The Web-UI also has a Licnese link here:
I have no appropriate media on my test instances but this looks to be a legitimate and long standing project, so lets get this Rock-on merged and out for general feedback. |
We also need a follow-up pull request to address our new normalisation regarding arch compatibility. @Hooverdan96 my apologies for muddying the waters on this front, but while we were on a merge run I wanted to not loose the already not insubstantial effort that has gone into this Rock-on preperation. @FroggyFlox From a quick look this pr does seem to have the required changes you requested. I know we have other requirements in the interim but again I'm keen to not loose the work we have here before it gets even older. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously reviewed by @FroggyFlox with a quick follow-up by myself.
All previously requested changes look now to have been applied, so lets get this one out for field testing.
We do need a follow-up pr to address arch compatibility however. Which we now add in our description.
This Rock-on has now been published to PRODUCTION. |
Fixes #109.
General information on project
This pull request proposes to add a new rock-on for the following project:
Information on docker image
node
tag: https://hub.docker.com/layers/rednoah/filebot/node/images/sha256-a6e993f96c2061cff168ccbf0efc26fac4f5f7305e4c4c8dce8fff8cf2b7c3b3?context=exploreChecklist
root.json
in alphabetical order (for new rock-on only)"description"
object lists and links to the docker image used"description"
object provides information on the image's particularities (advantage over another existing rock-on for the same project, for instance)"website"
object links to project's main website