Skip to content
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

Update Handbrake.json #367

Merged
merged 2 commits into from Mar 12, 2024
Merged

Update Handbrake.json #367

merged 2 commits into from Mar 12, 2024

Conversation

Hooverdan96
Copy link
Member

@Hooverdan96 Hooverdan96 commented Mar 10, 2024

  • add optical drive
  • change storage label
  • clean up comments

Fixes #366 .

Checklist

  • Passes JSONlint validation
  • "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

add optical drive
change storage label
clean up comments
remove html tags from info texts
change volume labels to better group different shares (sorted by label name
@Hooverdan96 Hooverdan96 self-assigned this Mar 11, 2024
@Hooverdan96 Hooverdan96 marked this pull request as ready for review March 11, 2024 18:45
@Hooverdan96
Copy link
Member Author

@phillxnet, @FroggyFlox, I have now cleaned up the original draft PR (e.g. removing html tags from the info texts, and adjusting the 'test' naming so it wouldn't conflict with the existing Rockon during testing).

If/when you get a chance to review, please kindly do so.

@Hooverdan96 Hooverdan96 added the needs review Test install, function, on / off behaviour, all links / info. label Mar 11, 2024
Copy link
Member

@phillxnet phillxnet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Hooverdan96 Thanks for seeing to these updates. As always much appreciated.

I only did a basic install to note the format etc.

I'm assuming the following was intended:
formatting-re-format-preset

My test install used the following settings:
handbrake-installed-config

The initial Web-UI was as follows (cut in height):
Initial-Handbrake-Web-UI

I did notice that the above Web-UI did not indicate the "Very Fast" as pre-selected; but "Fast ...".

No showstoppers from what I can tell so merging. Thanks again for your Rock-on maintenance: always much appreciated.

@phillxnet phillxnet merged commit d5f712d into rockstor:master Mar 12, 2024
@phillxnet
Copy link
Member

PR product PRODUCTION published.

@Hooverdan96
Copy link
Member Author

@phillxnet, on the above pieces, yes, that was intended, since we don't have an "optional/use default" setup for those parameters yet, so they're still required or installation won't proceed. And, if I remember correctly during the initial conversation on Handbrake some years ago, I think the main user had intended on being able to change the defaults ...

I think there will be a few Rockons that will benefit from the ability to make more parameters options (and possibly with a default value behind them, so even if they're mandatory they would then be pre-populated.

@Hooverdan96 Hooverdan96 deleted the 366_handbrak branch March 12, 2024 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs review Test install, function, on / off behaviour, all links / info.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handbrake Rockon in need of maintenance/deletion
2 participants