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

PD-1369 / 25.04 / Pd 1369 update custom app #3123

Merged
merged 92 commits into from
Oct 17, 2024

Conversation

DjP-iX
Copy link
Contributor

@DjP-iX DjP-iX commented Oct 10, 2024

Thanks for contributing to TrueNAS documentation! By opening a Pull Request, you're acknowledging that your changes will be distributed under the Creative Commons 4.0 license.

@DjP-iX DjP-iX marked this pull request as ready for review October 16, 2024 20:01
@DjP-iX DjP-iX requested a review from a team as a code owner October 16, 2024 20:01
@DjP-iX DjP-iX requested a review from stavros-k October 16, 2024 20:01
Copy link
Contributor

@micjohnson777 micjohnson777 left a comment

Choose a reason for hiding this comment

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

Found some small style stuff and minor correction suggestions.
Also the UI ref content is reading too much like tutorial content. in a later version of the Custom App UI ref we should clean this up.
We should try to reuse the app snippets where possible. Again, we can review the tutorial content for opportunities to use she snippets where it fits. The tutorial appears to just be explaining how to populate the fields like the UI ref does. Suggest in a future version we keep this if they are required to deploy the app. Again this can be done at a later date.

Finally, and most important..... EXCELLENT work!

content/TruenasApps/UsingCustomApp.md Show resolved Hide resolved
content/TruenasApps/UsingCustomApp.md Outdated Show resolved Hide resolved
content/TruenasApps/_index.md Outdated Show resolved Hide resolved
content/TruenasApps/_index.md Outdated Show resolved Hide resolved
content/TruenasApps/_index.md Outdated Show resolved Hide resolved
@micjohnson777 micjohnson777 added the Technical Writer Review (MJ) A technical writer is reviewing the PR. label Oct 16, 2024
@micjohnson777
Copy link
Contributor

micjohnson777 commented Oct 17, 2024 via email

@DjP-iX
Copy link
Contributor Author

DjP-iX commented Oct 17, 2024

Found some small style stuff and minor correction suggestions. Also the UI ref content is reading too much like tutorial content. in a later version of the Custom App UI ref we should clean this up. We should try to reuse the app snippets where possible. Again, we can review the tutorial content for opportunities to use she snippets where it fits. The tutorial appears to just be explaining how to populate the fields like the UI ref does. Suggest in a future version we keep this if they are required to deploy the app. Again this can be done at a later date.

Finally, and most important..... EXCELLENT work!

I think I got some of this in the comments along the way, but to reiterate:

I see what you're saying about the UI ref reading a bit like tutorial content but I was trying not to reinvent the wheel where there was material I could reuse from the previous version. Like you said, we can address that next time we do a pass over this content.

I did try to use the app snippets, but in most cases they were written in a way that makes sense for catalog apps but doesn't apply to custom apps, had information that is relevant to catalog apps but doesn't apply to custom apps, or lacks Docker-specific information that doesn't really matter to the average user just installing catalog apps but does when installing a custom app. I did mine them for some language but I thought it would've made the custom app tutorial more confusing to use them as is.

And as far as the tutorial generally explaining how to fill in the fields and not leaving out fields that are not needed to install the app, I don't see how we can do any differently. We don't know which app they're installing or what fields are or are not relevant to deploy any given app. If I was looking for a tutorial for how to use the custom app wizard, I'd be looking for something that explains how it all works so I can decide how to adapt whatever app I'm attempting to install to those fields.

micjohnson777
micjohnson777 previously approved these changes Oct 17, 2024
Copy link
Contributor

@micjohnson777 micjohnson777 left a comment

Choose a reason for hiding this comment

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

Ready to go. All changes render correctly! Excellent work!

@micjohnson777 micjohnson777 merged commit 062363a into master Oct 17, 2024
3 checks passed
@micjohnson777 micjohnson777 deleted the PD-1369-update-custom-app branch October 17, 2024 20:00
@bugclerk
Copy link
Contributor

This PR has been merged and conversations have been locked.
If you would like to discuss more about this issue please use our forums or raise a Jira ticket.

@truenas truenas locked as resolved and limited conversation to collaborators Oct 17, 2024
@DjP-iX
Copy link
Contributor Author

DjP-iX commented Oct 22, 2024

backport

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants