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

Add instructions for building and testing with Rust #35725

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

foursixnine
Copy link

@foursixnine foursixnine commented Dec 19, 2024

Right now, it's a manual search and replace, I would like to build the docs as I find myself the answers to enable my own rust workflows beyond what https://github.com/foursixnine/cndk8/blob/main/.github/workflows/rust.yml is already doing.

was initially: #35721 but panik.

PoC: Add documentation for rust workflows

Why:

Because there's no documentation on how to properly handle rust stuff on github

would take a look later at github/docs/issues/23181 and see what else fails

What's being changed (if available, include any code snippets, screenshots, or gifs):

So far adding documentation based off the Golang one, and my own journey through https://github.com/foursixnine/cndk8/ and isototest-ng

Commits, PR's will be listed

Check off the following:

  • Update filename
  • A subject matter expert (SME) has reviewed the technical accuracy of the content in this PR.
  • Review by SME from Github - In most cases, the author can be the SME. Open source contributions may require a SME review from GitHub staff.
  • The changes in this PR meet the docs fundamentals that are required for all content.
  • All CI checks are passing.

@github-actions github-actions bot added the triage Do not begin working on this issue until triaged by the team label Dec 19, 2024
@foursixnine foursixnine changed the title Oxidizegithub Add instructiuons for building and testing rust Dec 19, 2024
@foursixnine foursixnine changed the title Add instructiuons for building and testing rust Add instructions for building and testing with Rust Dec 20, 2024
@nguyenalex836
Copy link
Contributor

@foursixnine Thanks so much for opening a PR! I'll get this triaged for review ✨Just a quick tip - we'd recommend opening an issue before raising a PR, especially for larger scale changes such as this one 💛 This will let our team determine how best to proceed with the concern / issue you are raising, thank you!

@nguyenalex836 nguyenalex836 added content This issue or pull request belongs to the Docs Content team actions This issue or pull request should be reviewed by the docs actions team waiting for review Issue/PR is waiting for a writer's review and removed triage Do not begin working on this issue until triaged by the team labels Dec 20, 2024
@foursixnine
Copy link
Author

@foursixnine Thanks so much for opening a PR! I'll get this triaged for review ✨Just a quick tip - we'd recommend opening an issue before raising a PR

absolutely! I'm behind on creating the issue, was getting: https://github.com/foursixnine/cndk8/actions/runs/12436473960 in shape before, so I'll get back to creating the issue over the weekend :) to not lose momentum!

especially for larger scale changes such as this one 💛 This will let our team determine how best to proceed with the concern / issue you are raising, thank you!

awesome! I might need guidance, as I'm only looking at only gh community

@nguyenalex836
Copy link
Contributor

@foursixnine Ah gotcha, thank you! ✨ Well I hope you get to have a restful and wonderful weekend 💛

@foursixnine
Copy link
Author

foursixnine commented Dec 22, 2024

@foursixnine Ah gotcha, thank you! ✨ Well I hope you get to have a restful and wonderful weekend 💛

@nguyenalex836 : A lot of rest is being achieved :D - created #35747 as the issue for this and following Pull requests (if any) related to this.

@nguyenalex836 nguyenalex836 linked an issue Dec 23, 2024 that may be closed by this pull request
5 tasks
Copy link
Contributor

@janbrasna janbrasna left a comment

Choose a reason for hiding this comment

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

It's referred as WIP/PoC in a few places (although not marked as "Draft" so it appears as ready for review…) therefore this may be preliminary, but to make sure it's tracked in case:


This guide shows you how to build, test, and publish a Rust package.

{% data variables.product.prodname_dotcom %}-hosted runners have a tools cache with preinstalled software, which includes the dependencies for Go. For a full list of up-to-date software and the preinstalled versions of Go, see [AUTOTITLE](/actions/using-github-hosted-runners/about-github-hosted-runners#preinstalled-software).
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the Go toolchain mention intentional?

Comment on lines 5 to 7
fpt: '*'
ghes: '*'
ghec: '*'
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI the tracking issue mentions:

"… so It can be reused further by at least community GH (i.e no self hosted runners for now or enterprise features)"

I'm not sure there would be any difference if provisioning the same runners, just felt it's worth noting it's applied to all versions here, while the PR is actually marked as a PoC mentioning only being verified for dotcom and standard runners.

Copy link
Author

Choose a reason for hiding this comment

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

what would be the correct options then? I don't know ftp, ghes or ghec... what would you suggest?

@foursixnine foursixnine marked this pull request as draft December 27, 2024 09:01
@foursixnine
Copy link
Author

foursixnine commented Dec 27, 2024

It's referred as WIP/PoC in a few places (although not marked as "Draft" so it appears as ready for review…) therefore this may be preliminary, but to make sure it's tracked in case:

Thanks for the review, marked as draft for now - will re-request reviews once final draft ins ready (I started with a copy of the golang one, so that's why go is all over the place :P; that said thanks for the review, some questions I had you're already pointing out missing pieces.

@foursixnine
Copy link
Author

Ok, https://crates.io/crates/cndk8/0.1.0 has been published using only github, now docfest starts :D https://github.com/foursixnine/cndk8/actions/runs/12544879009/job/34978215202

@felicitymay felicitymay added the needs SME This proposal needs review from a subject matter expert label Jan 2, 2025
Copy link
Contributor

github-actions bot commented Jan 2, 2025

Thanks for opening a pull request! We've triaged this issue for technical review by a subject matter expert 👀

Comment on lines 118 to 123
- name: Publish hello app to GitHub Packages
run: |
curl -u "${{ github.actor }}:${{ secrets.GH_TOKEN }}" \
-X POST "https://uploads.github.com/repos/${{ github.repository }}/releases/assets?name=cndk8-hello.tar.gz" \
--header "Content-Type: application/gzip" \
--data-binary @./cndk8-hello/cndk8
Copy link
Author

Choose a reason for hiding this comment

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

I wonder if this is wanted/desired or if gh has plans to offer a rust registry

…ng/building-and-testing-rust.md

Co-authored-by: Matt Rogers <[email protected]>
…ing-and-testing/building-and-testing-rust.md

Co-authored-by: Matt Rogers <[email protected]>
@foursixnine
Copy link
Author

@mattr- Just requested re-review, let me know if anything else is needed, and if I should do squash of some of the commits TIA!

@nguyenalex836 nguyenalex836 added SME reviewed An SME has reviewed this issue/PR and removed waiting for review Issue/PR is waiting for a writer's review needs SME This proposal needs review from a subject matter expert labels Jan 16, 2025
Copy link

@mattr- mattr- left a comment

Choose a reason for hiding this comment

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

Thanks for making those updates! I found one other thing that I noticed the first read through but failed to write a comment on. Other than that one thing, this is ready to go.

Comment on lines 118 to 122
run: |
curl -u "${{ github.actor }}:${{ secrets.GITHUB_TOKEN }}" \
-X POST "https://uploads.github.com/repos/${{ github.repository }}/releases/assets?name=cndk8-hello.tar.gz" \
--header "Content-Type: application/gzip" \
--data-binary @./cndk8-hello/cndk8
Copy link

Choose a reason for hiding this comment

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

I missed commenting on this the first time through. I haven't been able to find documentation about uploads.github.com anywhere other than some stuff for GitHub Connect, which I don't think would be applicable for this guide. What do you think about switching this to use something like gh release upload instead?

Copy link
Author

@foursixnine foursixnine Jan 16, 2025

Choose a reason for hiding this comment

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

What do you think about switching this to use something like gh release upload instead?

@mattr- I feel we could remove this all together; adding a step that does gh release upload, goes more into the question I had already in this comment of whether there would/could be a github registry for rust packages or if this is needed at all, in the context of the guide...

Copy link
Author

Choose a reason for hiding this comment

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

@foursixnine foursixnine requested a review from mattr- January 17, 2025 12:18
@foursixnine
Copy link
Author

I found one other thing that I noticed the first read through but failed to write a comment on.

Just addressed it, ended up being easier than I thought :D

Other than that one thing, this is ready to go.

🚀🚀🚀 happy to read that, lmk if anything else is needed.

Copy link
Contributor

@janbrasna janbrasna left a comment

Choose a reason for hiding this comment

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

Thinking out loud: (mostly notes for staff/reviewer to consider if applicable…)

path: ./cndk8-hello
- name: Publish hello app binary as a release on GitHub
- run: |
gh release create v0.1.0 --generate-notes
Copy link
Contributor

Choose a reason for hiding this comment

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

the hard-coded tag doesn't make a good example as folks would trip over this when used in a real workflow — gh release can use some sensible defaults of adding artifacts to the most recent release IIRC?

Copy link
Contributor

Choose a reason for hiding this comment

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

(TBH I'd avoid creating the release in CI at all; and instead publish the artifacts on release trigger — attaching the assets to the recent/created release instead?)

- name: Publish hello app binary as a release on GitHub
- run: |
gh release create v0.1.0 --generate-notes
gh release upload v0.1.0 ./cndk8-hello/cndk8#my-cndk8-hello-app
Copy link
Contributor

Choose a reason for hiding this comment

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

the paths seem too specific? (probably taken from a real project, should use some simplifying for demonstration here)

Comment on lines +116 to +117
name: cndk8-hello
path: ./cndk8-hello
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't make it obvious why this folder structure and naming, would something like hello-app or an obvious placeholder be a better example?

run: cargo publish # publishes your crate as a library that can be added as a dependency
```

As an example of how packages are published, see the [cndk8 0.1.0](https://crates.io/crates/cndk8/0.1.0). In the case that there are errors with Metadata check
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure if linking to a non-example crate on an external site is sustainable to maintain from docs; don't see the utility of the reference.

(this all seems taken from a real-world workflow of a real-world project — following the other CI tutorials it would feel better as an "anonymized" hello world type of code and naming?

Copy link
Contributor

Choose a reason for hiding this comment

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

e.g. in the case of .NET that needs an argument with some placeholder filled in, it uses the GH Docs convention of using angle brackets:

run: dotnet pack --configuration Release <my project>

The examples here should too make it obvious what are just placeholder values in need of filling in correctly before the examples start to work for the reader…

path: target/${{ matrix.BUILD_TARGET }}/cndk8
```

And to use them on a different job, i.e publishing as a github release, check [AUTOTITLE](/actions/security-for-github-actions/security-guides/automatic-token-authentication) to ensure your workflows have the right permissions on the repository.
Copy link
Contributor

Choose a reason for hiding this comment

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

"GitHub Release" is the correct case — product names should come from a variable(?)

1. Edit the workflow as required. For example, change the version of Rust.
1. Click **Commit changes**.

{% ifversion fpt or ghec %}
Copy link
Contributor

Choose a reason for hiding this comment

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

TODO: check for versions applicability.

title: Building and testing Rust
intro: You can create a continuous integration (CI) workflow to build and test your Rust project.
versions:
ghec: '*'
Copy link
Contributor

Choose a reason for hiding this comment

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

TODO: check for versions applicability.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is also the reason you don't "see" it in the overview page — cuz the default would be fpt, and since you don't spec that, you can only see it if you switch the overview to ghec — otherwise it won't get listed there.

- CI
shortTitle: Build, test & Publish with Rust
---
<!-- {% data reusables.actions.enterprise-github-hosted-runners %}-->
Copy link
Contributor

Choose a reason for hiding this comment

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

TODO: check for versions applicability.

And either include the reusable if relevant; or don't if not needed anymore.

type: tutorial
topics:
- CI
shortTitle: Build, test & Publish with Rust
Copy link
Contributor

Choose a reason for hiding this comment

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

There are three places, title, intro, shortTitle, that either mention publishing, or don't. Consistency?

NB: Correct sentence case?

Copy link
Contributor

Choose a reason for hiding this comment

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

TODO: There's a style guide for UI highlights — does the orange rectangle follow the guidelines for consistency?

TODO: Size/dimensions of the image, is it expected to follow some resolution pattern from other pages for legibility?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
actions This issue or pull request should be reviewed by the docs actions team content This issue or pull request belongs to the Docs Content team SME reviewed An SME has reviewed this issue/PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rust: Add instructions for building and testing
5 participants