-
Notifications
You must be signed in to change notification settings - Fork 31
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 Plex Rockon Documentation #499
Conversation
Fixes rockstor#214 and rockstor#172 . Update of Plex Rockon Documentation: - newer screenshots - refactoring descriptions - include transcoding explanation - add workaround when no Plex account is used - [X] With the proposed changes no Sphinx errors or warnings are generated. - [X] I have added my name to the AUTHORS file, if required (descending alphabetical order).
These are the three linkcheck failures:
However, when manually accessing these three, they come up fine (no visible redirects or other address changes). |
@FroggyFlox, @phillxnet I'm sure I missed some stuff, please take a look and make suggestions for change. |
@Hooverdan96 Just having a looks at this now:
So this may be a candidate, when re-visiting, to add our ignored list as we have the two you posted here already to add also. Just reviewing changes currently, and this is not a show-stopper so no worries really. Just wanted to add to the known flaky urls as and when we see them. [EDIT] My apologies: I see you already listed this!! |
@Hooverdan96 Just a question up-front: why was your indicated Plex user, also a Rockstor Web-UI admin user? Also was the following an intentional formatting: [EDIT] I've popped in a suggested fix (hopefully) for the link format via the now completed review. |
The admin user was not planned. I just had it created in that "demo" instance that way. I can clarify (or create a different screenshot for that. |
@phillxnet, I rebased the fork based on your most recent merge, updated the User screenshot and fixed the link you mentioned above. |
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.
@Hooverdan96 This is a nice improvement on what we had which was now a little dated. As always much appreciated.
I've made a few suggested improvements via the review. But my main concern here is the use of an Admin user (in picture); when text suggests non-admin user: which I think is preferred. Plus the share owner and group pic/text anomaly similarly. That is a regression re-setup I think.
See what you think.
That would be preferred if you have the time, thanks. That way we get folks using what is a safer arrangement from the get-go.
No worries, I've suggested a fix in the review. I hope the review suggestions are still valid as you look to have re-based and modifieed while I was reviewing. I can always re-do if the suggested changes no longer tally. |
Co-authored-by: Philip Guyton <[email protected]>
Co-authored-by: Philip Guyton <[email protected]>
Co-authored-by: Philip Guyton <[email protected]>
Co-authored-by: Philip Guyton <[email protected]>
Co-authored-by: Philip Guyton <[email protected]>
@phillxnet, ok I think I have now captured everything I can think of, including all of your suggestions. |
@Hooverdan96 The only thing I see amiss after these changes is that the plex_share_owner.png file still looks to be root.root with default settings. Should this not be plex.plex with no 'other' access? With this share config Plex would have no write access. As I suspect you have the image there, do you fancy popping that in on this PR. We can do anther time: I'm just not keen of publishing as-is given the no write access and non optimal owner.group. Given the text is all nice now, and we have the new non-admin plex user image. |
no, you're right, I thought I had done it ... starting too long at your own writing/pictures makes you overlook crucial things. I will update the view. |
@phillxnet, ok I misread one of your earlier suggestions and only associated that with the plex user being part of the admin group. The share owner under the Access Control tab screenshot is now updated as well. |
@Hooverdan96 Thanks again for all this effort, this is a major improvement and it's nice to now have this key guide updated re the transcoding option/Share. And to have more refined explanations etc. About to merge and get this published shortly. |
PR product PRODUCTION published. |
Fixes #214 and #172 .
Update of Plex Rockon Documentation:
newer screenshots
refactoring descriptions
include transcoding explanation
add workaround when no Plex account is used
With the proposed changes no Sphinx errors or warnings are generated.
I have added my name to the AUTHORS file, if required (descending alphabetical order).