-
Notifications
You must be signed in to change notification settings - Fork 615
Initial (GHA) Workflow for creating the Porting Guide #121
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
base: devel
Are you sure you want to change the base?
Changes from all commits
ab35165
8288761
fcb1258
1b634de
1167701
7a367da
a684ca6
c7ae2d3
c873ac3
52096c5
edc88cf
d9206c7
f5c5813
8588946
aea1067
e1bfd19
dc77ee1
8f9cd1b
97133cd
68ed0ee
8eeeac5
1ada5a3
02478f4
695f1a4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,54 @@ | ||||||||||||||||||||||||||||||||||
--- | ||||||||||||||||||||||||||||||||||
name: Ansible porting guide | ||||||||||||||||||||||||||||||||||
on: | ||||||||||||||||||||||||||||||||||
workflow_dispatch: | ||||||||||||||||||||||||||||||||||
inputs: | ||||||||||||||||||||||||||||||||||
ansible-version: | ||||||||||||||||||||||||||||||||||
description: Release version. For example, 11.1.0 | ||||||||||||||||||||||||||||||||||
required: true | ||||||||||||||||||||||||||||||||||
ansible-major-version: | ||||||||||||||||||||||||||||||||||
description: Major version. For example, 11 | ||||||||||||||||||||||||||||||||||
required: true | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
jobs: | ||||||||||||||||||||||||||||||||||
upload-porting-guide: | ||||||||||||||||||||||||||||||||||
name: Extract the porting guide${{ '' }} # Nest jobs under the same sidebar category | ||||||||||||||||||||||||||||||||||
runs-on: ubuntu-latest | ||||||||||||||||||||||||||||||||||
env: | ||||||||||||||||||||||||||||||||||
package-url: https://files.pythonhosted.org/packages/source/a/ansible | ||||||||||||||||||||||||||||||||||
ansible-tarball: ansible-${{ inputs.ansible-version }}.tar.gz | ||||||||||||||||||||||||||||||||||
extracted-path: ansible-${{ inputs.ansible-version }} | ||||||||||||||||||||||||||||||||||
porting-guide-rst: porting_guide_${{ inputs.ansible-major-version }}.rst | ||||||||||||||||||||||||||||||||||
Comment on lines
+18
to
+21
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Environment variables should always be written upper case and use
Suggested change
Suggested change
|
||||||||||||||||||||||||||||||||||
steps: | ||||||||||||||||||||||||||||||||||
- name: Fetch and unpack the package tarball | ||||||||||||||||||||||||||||||||||
run: >- | ||||||||||||||||||||||||||||||||||
wget ${{ env.package-url }}/${{ env.ansible-tarball }} | ||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In shell scripts, always use
Suggested change
|
||||||||||||||||||||||||||||||||||
&& | ||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why use If a command fails, the whole step fails anyway. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
@felixfontein that's actually not universally true. There are corner cases (mostly for Windows runners IIRC) where the second command would still get executed, and its return code would be used for the step status. To fight this, either boilerplate like |
||||||||||||||||||||||||||||||||||
tar -xvf ${{ env.ansible-tarball }} | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
- name: Create an artifact with the RST file | ||||||||||||||||||||||||||||||||||
uses: actions/upload-artifact@v4 | ||||||||||||||||||||||||||||||||||
with: | ||||||||||||||||||||||||||||||||||
name: ansible-porting-guide | ||||||||||||||||||||||||||||||||||
path: ${{ env.extracted-path }}/${{ env.porting-guide-rst }} | ||||||||||||||||||||||||||||||||||
retention-days: 7 | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
create-docs-pr: | ||||||||||||||||||||||||||||||||||
name: Create a docs PR${{ '' }} # Nest jobs under the same sidebar category | ||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is only necessary to use the
Suggested change
|
||||||||||||||||||||||||||||||||||
needs: upload-porting-guide | ||||||||||||||||||||||||||||||||||
uses: ./.github/workflows/reusable-porting-guide.yml | ||||||||||||||||||||||||||||||||||
with: | ||||||||||||||||||||||||||||||||||
repository: ansible/ansible-documentation | ||||||||||||||||||||||||||||||||||
path: ansible-documentation | ||||||||||||||||||||||||||||||||||
ansible-version: ${{ inputs.ansible-version }} | ||||||||||||||||||||||||||||||||||
ansible-major-version: ${{ inputs.ansible-major-version }} | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
create-build-data-pr: | ||||||||||||||||||||||||||||||||||
name: Create a build-data PR${{ '' }} # Nest jobs under the same sidebar category | ||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||||
needs: upload-porting-guide | ||||||||||||||||||||||||||||||||||
uses: ./.github/workflows/reusable-porting-guide.yml | ||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is this step for? Why does a PR in ansible-build-data needs to be created? |
||||||||||||||||||||||||||||||||||
with: | ||||||||||||||||||||||||||||||||||
repository: ansible-community/ansible-build-data | ||||||||||||||||||||||||||||||||||
path: ansible-build-data | ||||||||||||||||||||||||||||||||||
ansible-version: ${{ inputs.ansible-version }} | ||||||||||||||||||||||||||||||||||
ansible-major-version: ${{ inputs.ansible-major-version }} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,96 @@ | ||||||||||||||||||||||||||||||||||||||||||
--- | ||||||||||||||||||||||||||||||||||||||||||
name: Create porting guide PR | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
"on": | ||||||||||||||||||||||||||||||||||||||||||
workflow_call: | ||||||||||||||||||||||||||||||||||||||||||
inputs: | ||||||||||||||||||||||||||||||||||||||||||
repository: | ||||||||||||||||||||||||||||||||||||||||||
type: string | ||||||||||||||||||||||||||||||||||||||||||
description: The repository to check out. | ||||||||||||||||||||||||||||||||||||||||||
required: true | ||||||||||||||||||||||||||||||||||||||||||
path: | ||||||||||||||||||||||||||||||||||||||||||
type: string | ||||||||||||||||||||||||||||||||||||||||||
description: The path for the repository. | ||||||||||||||||||||||||||||||||||||||||||
required: true | ||||||||||||||||||||||||||||||||||||||||||
ansible-version: | ||||||||||||||||||||||||||||||||||||||||||
type: string | ||||||||||||||||||||||||||||||||||||||||||
description: Ansible release version. | ||||||||||||||||||||||||||||||||||||||||||
required: true | ||||||||||||||||||||||||||||||||||||||||||
ansible-major-version: | ||||||||||||||||||||||||||||||||||||||||||
type: string | ||||||||||||||||||||||||||||||||||||||||||
description: Ansible major version. | ||||||||||||||||||||||||||||||||||||||||||
required: true | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
env: | ||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Technically, environment variables should be referenceable in Bash and therefore shouldn't contain dashes. But keep using dashes in the GHA-only vars (like inputs/outputs and job/step IDs). |
||||||||||||||||||||||||||||||||||||||||||
git-branch: porting-guide-${{ inputs.ansible-version }} | ||||||||||||||||||||||||||||||||||||||||||
porting-guide-rst: porting_guide_${{ inputs.ansible-major-version }}.rst | ||||||||||||||||||||||||||||||||||||||||||
ci-commit-message: >- | ||||||||||||||||||||||||||||||||||||||||||
Add the Ansible community "${{ inputs.ansible-version }}" porting guide | ||||||||||||||||||||||||||||||||||||||||||
pr-body-message: >- | ||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use the pipe syntax, since
Suggested change
Tip See https://yaml-multiline.info to experiment and see how different multiline string block scalars are rendered. |
||||||||||||||||||||||||||||||||||||||||||
##### SUMMARY | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
Add the Ansible "${{ inputs.ansible-major-version }}" porting guide. | ||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+28
to
+32
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There should be no quotes used here. We do not have an
Suggested change
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
##### ISSUE TYPE | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
- Docs Pull Request | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
##### COMPONENT NAME | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
porting_guide_${{ inputs.ansible-major-version }}.rst | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
jobs: | ||||||||||||||||||||||||||||||||||||||||||
create-pr: | ||||||||||||||||||||||||||||||||||||||||||
runs-on: ubuntu-latest | ||||||||||||||||||||||||||||||||||||||||||
steps: | ||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since all the steps need the same working dir, you can just change the default.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||
- name: Check out the repository | ||||||||||||||||||||||||||||||||||||||||||
uses: actions/checkout@v4 | ||||||||||||||||||||||||||||||||||||||||||
with: | ||||||||||||||||||||||||||||||||||||||||||
repository: ${{ inputs.repository }} | ||||||||||||||||||||||||||||||||||||||||||
path: ${{ inputs.path }} | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
- name: Download the artifact with the RST file | ||||||||||||||||||||||||||||||||||||||||||
uses: actions/download-artifact@v4 | ||||||||||||||||||||||||||||||||||||||||||
with: | ||||||||||||||||||||||||||||||||||||||||||
name: ansible-porting-guide | ||||||||||||||||||||||||||||||||||||||||||
path: ${{ inputs.path }} | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
- name: Copy the RST file to the docs repo | ||||||||||||||||||||||||||||||||||||||||||
if: ${{ inputs.repository == 'ansible/ansible-documentation' }} | ||||||||||||||||||||||||||||||||||||||||||
run: cp -v ${{ env.porting-guide-rst }} docs/docsite/rst/porting_guides/ | ||||||||||||||||||||||||||||||||||||||||||
working-directory: ${{ inputs.path }} | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
- name: Copy the RST file to the build-data repo | ||||||||||||||||||||||||||||||||||||||||||
if: ${{ inputs.repository == 'ansible-community/ansible-build-data' }} | ||||||||||||||||||||||||||||||||||||||||||
run: cp -v ${{ env.porting-guide-rst }} ${{ inputs.ansible-major-version }} | ||||||||||||||||||||||||||||||||||||||||||
working-directory: ${{ inputs.path }} | ||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+58
to
+66
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These steps are almost identical. Perhaps, it's best to collapse these into a single step, with the target computed based on inputs..
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
- name: Set up git | ||||||||||||||||||||||||||||||||||||||||||
run: | | ||||||||||||||||||||||||||||||||||||||||||
git checkout -b "${{ env.git-branch }}" | ||||||||||||||||||||||||||||||||||||||||||
git config --global user.name "${{ github.actor }}" | ||||||||||||||||||||||||||||||||||||||||||
git config --global user.email "${{ github.actor }}@users.noreply.github.com" | ||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Put things that aren't in an environment variable into one for this step, and use envrionment variables. Otherweise there is no sane and safe way to use these in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. a proper email should contain the account id too FYI |
||||||||||||||||||||||||||||||||||||||||||
working-directory: ${{ inputs.path }} | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
- name: Add the porting guide | ||||||||||||||||||||||||||||||||||||||||||
run: git add . --all --force | ||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't use I'd also recommend being more granular and committing what's expected to exist + adding an assertion that no uncommitted files left. |
||||||||||||||||||||||||||||||||||||||||||
working-directory: ${{ inputs.path }} | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
- name: Commit the porting guide | ||||||||||||||||||||||||||||||||||||||||||
run: >- | ||||||||||||||||||||||||||||||||||||||||||
git diff-index --quiet HEAD || | ||||||||||||||||||||||||||||||||||||||||||
git commit -m "${{ env.ci-commit-message }}" | ||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Especially this use is extremely dangerous. Never use GHA interpolation inside |
||||||||||||||||||||||||||||||||||||||||||
working-directory: ${{ inputs.path }} | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
- name: Push to the repo | ||||||||||||||||||||||||||||||||||||||||||
run: git push origin "${{ env.git-branch }}" | ||||||||||||||||||||||||||||||||||||||||||
working-directory: ${{ inputs.path }} | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
- name: Create the porting guide PR | ||||||||||||||||||||||||||||||||||||||||||
run: >- | ||||||||||||||||||||||||||||||||||||||||||
gh pr create | ||||||||||||||||||||||||||||||||||||||||||
--base test | ||||||||||||||||||||||||||||||||||||||||||
--head "publish-${{ inputs.ansible-version }}" | ||||||||||||||||||||||||||||||||||||||||||
--title "${{ env.ci-commit-message }}" | ||||||||||||||||||||||||||||||||||||||||||
--body "${{ env.pr-body-message }}" | ||||||||||||||||||||||||||||||||||||||||||
working-directory: ${{ inputs.path }} | ||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+90
to
+96
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The working directory is irrelevant here, FYI. How about implementing something like pytest-dev/pytest#12502 to improve the UX? |
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.
Why is that
${{ '' }}
needed, and what does the comment wants to say? Does the comment refer to the templating, or to something else?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.
@felixfontein it's unnecessary in this specific instance. But in general, it's a hack I invented to fight how poorly GHA is rendering matrixes of reusable workflows in the sidebar by default (with multiple collapsed sections for each of the jobs within a matrix).