-
Notifications
You must be signed in to change notification settings - Fork 452
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
docs: port the platform integration guides from Discourse #5160
docs: port the platform integration guides from Discourse #5160
Conversation
@mr-cal I wanted to give you a preview of the shape, look, and feel of these pages. I aim to have this ready for a full review by end of pulse. If you have high-level concerns, please let me know. A few things to note:
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5160 +/- ##
==========================================
- Coverage 94.88% 89.71% -5.17%
==========================================
Files 658 342 -316
Lines 55189 22638 -32551
==========================================
- Hits 52364 20309 -32055
+ Misses 2825 2329 -496 ☔ View full report in Codecov by Sentry. |
@mr-cal I need to separate the example files, but otherwise this should be ready to review. |
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.
Nice work, from a high level the structure and design is great.
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.
I noticed some plugin reference documentation links point to snapcraft.io and others point to RTD. Is that intentional?
This is what I am ignoring in this review:
- Some guides use core18 which is technically still supported in snapcraft 7 but newer examples would be preferred. For example, the Rust guide uses core18.
- I am assuming the examples work as they aren't tested and I didn't test them.
Correct. There's no way around this problem at the moment. It's an ugliness we'll have to live with while the migration is in progress. |
Merging this will unblock canonical/open-documentation-academy#9. |
1684dd9
to
27c1360
Compare
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.
The spelling of wethr
needs fixing, but the other suggestions can wait until later.
Co-authored-by: Alex Lowe <alex.lowe@canonical.com>
Co-authored-by: Alex Lowe <alex.lowe@canonical.com>
codespell didn't flag the comment locally, but oddly did in the runner..
tox run -m lint
?tox run -e test-py310
? (supported versions:py39
,py310
,py311
,py312
)CRAFT-3577