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

docs: port the platform integration guides from Discourse #5160

Merged

Conversation

medubelko
Copy link
Contributor

@medubelko medubelko commented Nov 29, 2024

  • Have you followed the guidelines for contributing?
  • Have you signed the CLA?
  • Have you successfully run tox run -m lint?
  • Have you successfully run tox run -e test-py310? (supported versions: py39, py310, py311, py312)

CRAFT-3577

@medubelko medubelko requested a review from mr-cal November 29, 2024 11:03
@medubelko
Copy link
Contributor Author

medubelko commented Nov 29, 2024

@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:

  • The longest file in the stack contains all the unused text and should be ignored. The smaller temp files contain text for the remaining pages.
  • Take note that I consider these to flawed how-tos. As we move files away from Discourse, there will be staging fragments of pages that don't have the right home in the quadrants yet. As we adopt more pages and the population starts to be properly partitioned, I'll be correcting these issues and shifting the offending text around.
  • The code isn't in separate files yet, but this won't matter much because the recipes won't be running on spread.

Copy link

codecov bot commented Nov 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.71%. Comparing base (654871d) to head (ba374e4).
Report is 647 commits behind head on main.

❗ There is a different number of reports uploaded between BASE (654871d) and HEAD (ba374e4). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (654871d) HEAD (ba374e4)
2 1
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.
📢 Have feedback on the report? Share it here.

@medubelko
Copy link
Contributor Author

@mr-cal I need to separate the example files, but otherwise this should be ready to review.

@medubelko medubelko marked this pull request as ready for review November 30, 2024 01:30
Copy link
Collaborator

@mr-cal mr-cal left a 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.

docs/howto/craft-for-platforms/index.rst Outdated Show resolved Hide resolved
docs/howto/craft-for-platforms/example-compiled-app.rst Outdated Show resolved Hide resolved
docs/howto/craft-for-platforms/example-python-app.rst Outdated Show resolved Hide resolved
@mr-cal mr-cal self-requested a review December 2, 2024 19:09
@medubelko medubelko requested a review from lengau December 3, 2024 19:05
Copy link
Collaborator

@mr-cal mr-cal left a 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.

docs/howto/craft-for-platforms/example-python-app.rst Outdated Show resolved Hide resolved
docs/howto/craft-for-platforms/example-dotnet-app.rst Outdated Show resolved Hide resolved
docs/howto/craft-for-platforms/example-flutter-app.rst Outdated Show resolved Hide resolved
docs/howto/craft-for-platforms/example-ros-2-app.rst Outdated Show resolved Hide resolved
docs/howto/craft-for-platforms/example-ros-2-app.rst Outdated Show resolved Hide resolved
docs/howto/craft-for-platforms/example-ros-2-app.rst Outdated Show resolved Hide resolved
@medubelko
Copy link
Contributor Author

@mr-cal

I noticed some plugin reference documentation links point to snapcraft.io and others point to RTD. Is that intentional?

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.

@medubelko medubelko requested a review from mr-cal December 4, 2024 20:35
@medubelko
Copy link
Contributor Author

Merging this will unblock canonical/open-documentation-academy#9.

@medubelko medubelko force-pushed the craft-3577-docs-framework-howto-migration branch from 1684dd9 to 27c1360 Compare December 5, 2024 18:41
Copy link
Contributor

@lengau lengau left a 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.

docs/howto/craft-for-platforms/example-node-app.rst Outdated Show resolved Hide resolved
setup.cfg Outdated Show resolved Hide resolved
docs/howto/craft-for-platforms/index.rst Show resolved Hide resolved
docs/howto/craft-for-platforms/example-c-or-cpp-app.rst Outdated Show resolved Hide resolved
docs/howto/craft-for-platforms/example-python-app.rst Outdated Show resolved Hide resolved
medubelko and others added 5 commits December 5, 2024 13:29
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..
@medubelko medubelko enabled auto-merge (squash) December 6, 2024 00:33
@medubelko medubelko merged commit dc96a6d into canonical:main Dec 6, 2024
14 of 15 checks passed
@medubelko medubelko deleted the craft-3577-docs-framework-howto-migration branch December 6, 2024 18:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants