-
-
Notifications
You must be signed in to change notification settings - Fork 41
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
Adds Technical Blueprint to pkgdown site #923
Conversation
Preview here: https://averissimo.github.io/teal/ Note that links to the source vignettes are dead (as they don't yet existing on |
Code Coverage Summary
Diff against main
Results for commit: 99ffacd Minimum allowed coverage is ♻️ This comment has been updated with latest results |
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.
couple small comments. Thanks for working on this!
vignettes/images/reporter.jpg
Outdated
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.
Is this a duplicate of blueprint/reporter.jpg
?
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.
Nicely done and ripe for finishing touches 👍
However, I find name thoroughly misleading. A blueprint is a design to be followed when building something, not an explanation of the inner workings of a mechanism.
There was a discussion in the issue about how to do that and I just wanted to point out that I really like the way you proposed here. It enables (some) quarto features that we need with minimal workarounds (e.g. import mermaid lib without e.g. modifying docs workflow). Nicely done! |
Co-authored-by: Pawel Rucki <[email protected]> Signed-off-by: Dony Unardi <[email protected]>
Co-authored-by: Aleksander Chlebowski <[email protected]> Signed-off-by: Dony Unardi <[email protected]>
Signed-off-by: Dony Unardi <[email protected]>
Co-authored-by: Aleksander Chlebowski <[email protected]> Signed-off-by: Dony Unardi <[email protected]>
Co-authored-by: Aleksander Chlebowski <[email protected]> Signed-off-by: Dony Unardi <[email protected]>
Co-authored-by: Aleksander Chlebowski <[email protected]> Signed-off-by: Dony Unardi <[email protected]>
Co-authored-by: Aleksander Chlebowski <[email protected]> Signed-off-by: Dony Unardi <[email protected]>
Co-authored-by: Aleksander Chlebowski <[email protected]> Signed-off-by: Dony Unardi <[email protected]>
Co-authored-by: Aleksander Chlebowski <[email protected]> Signed-off-by: Dony Unardi <[email protected]>
Co-authored-by: Aleksander Chlebowski <[email protected]> Signed-off-by: Dony Unardi <[email protected]>
Co-authored-by: Aleksander Chlebowski <[email protected]> Signed-off-by: Dony Unardi <[email protected]>
Co-authored-by: Aleksander Chlebowski <[email protected]> Signed-off-by: Dony Unardi <[email protected]>
Co-authored-by: Pawel Rucki <[email protected]> Signed-off-by: Dony Unardi <[email protected]>
I enjoyed going through the Technical blueprint vignettes. I have a few comments:
Please note that these are just suggestions that are not intended to block this PR, I'm happy to create a new discussion issue for this. |
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.
@vedhav
Thanks for the review.
This is meant for teal
Technical Blueprint, not so much about the technical
aspect on collaboration (we have a different md for that). We could change the title to teal
Technical Blueprint on the menu bar, but I wonder if it would seem repeated.
These articles are meant to cover teal concepts at a high-level. Any deeper knowledge of a topic should be directed to the respective R packages. I changed some wording around qenv
and added a link to creating custom module vignettes to see qenv
usage. If we need a diagram about this, maybe we can add it here?
Finally, can I ask your assistance to review these suggested changes?
Co-authored-by: Dony Unardi <[email protected]> Signed-off-by: Vedha Viyash <[email protected]>
Co-authored-by: Dony Unardi <[email protected]> Signed-off-by: Vedha Viyash <[email protected]>
Co-authored-by: Dony Unardi <[email protected]> Signed-off-by: Vedha Viyash <[email protected]>
Co-authored-by: Dony Unardi <[email protected]> Signed-off-by: Vedha Viyash <[email protected]>
Pull Request
Fixes https://github.com/insightsengineering/coredev-tasks/issues/467
Preview
https://averissimo.github.io/teal/index.html
(deployed to personal repo)
Problems to solve
Changes description
Screenshot