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.

Sign up for GitHub

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

Jump to bottom

chore: Added updating of docs site with compat table #2205

Merged
merged 4 commits into from
May 28, 2024
Merged

chore: Added updating of docs site with compat table #2205

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
75ecad1
chore: Added updating of docs site with compat table
jsumners-nr May 21, 2024
c8120ea
run when package lock changes
jsumners-nr May 22, 2024
6a0d0c5
utilize action to detect dependency changes
jsumners-nr May 23, 2024
ff9df16
no need to lint a non-existent lockfile
jsumners-nr May 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 28 additions & 38 deletions .github/workflows/ci-workflow.yml
View file
Open in desktop
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I simplified this workflow while I was at it (mainly to fix it so that I didn't have to wait on all the runs while working on this PR). Turns out, I had a typo that was breaking things (output should be outputs). But when I really reviewed it, I realized that the "skip release" was really trying to accomplish the same task. So I refactored things a bit and now we should only need the one conditional check (if it works as I intend).

Original file line number Diff line number Diff line change
Expand Up @@ -8,43 +8,40 @@ env:

jobs:
should_run:
# We only want the test suites to run when code has changed, or when
# a dependency update has occurred.
runs-on: ubuntu-latest
permissions:
pull-requests: read
outputs:
javascript_changed: ${{ steps.filter.output.javascript }}
javascript_changed: ${{ steps.filter.outputs.javascript }}
deps_changed: ${{ steps.deps.outputs.divergent }}
steps:
- uses: actions/checkout@v4
with:
fetch-depth: 2
- uses: dorny/paths-filter@v3
id: filter
with:
filters: |
javascript:
- '**/*.js'
Copy link
Member

Choose a reason for hiding this comment

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

verify nice

Copy link
Member

Choose a reason for hiding this comment

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

it just occurred to me that if we only update a dep this CI won't run. but that would also cause the prep release PR to run because the package.json will change as well so maybe we need both?

- '**/*.json'
- '**/*.mjs'
- '**/*.cjs'

skip_if_release:
runs-on: ubuntu-latest

outputs:
should_skip: ${{ steps.skip_check.outputs.should_skip }}

steps:
- id: skip_check
uses: fkirc/skip-duplicate-actions@v5
- 'api.js'
- 'esm-loader.mjs'
- 'index.js'
- 'stub_api.js'
- 'lib/**/*.{js,json,mjs,cjs}'
- 'test/**/*.{js,json,mjs,cjs}'
bizob2828 marked this conversation as resolved.
Show resolved Hide resolved
- uses: jsumners-nr/gha-node-deps-divergent@643628fe0da51ec025e984c4644f17fd9f9e93f6
id: deps
with:
paths_ignore: '["NEWS.md", "changelog.json", "package.json", "package-lock.json"]'
skip_after_successful_duplicate: false
do_not_skip: '["workflow_dispatch", "pull_request"]'
base-sha: ${{ github.base_ref }}
current-sha: ${{ github.sha }}

lint:
needs:
- skip_if_release
- should_run
if: needs.skip_if_release.outputs.should_skip != 'true'
|| needs.should_run.outputs.javascript_changed == 'true'
if: needs.should_run.outputs.javascript_changed == 'true' ||
needs.should_run.outputs.deps_changed == 'true'
runs-on: ubuntu-latest

strategy:
Expand All @@ -61,15 +58,12 @@ jobs:
run: npm install
- name: Run Linting
run: npm run lint
- name: Inspect Lockfile
run: npm run lint:lockfile

ci:
needs:
- skip_if_release
- should_run
if: needs.skip_if_release.outputs.should_skip != 'true'
|| needs.should_run.outputs.javascript_changed == 'true'
if: needs.should_run.outputs.javascript_changed == 'true' ||
needs.should_run.outputs.deps_changed == 'true'
runs-on: ubuntu-latest

strategy:
Expand All @@ -90,10 +84,9 @@ jobs:

unit:
needs:
- skip_if_release
- should_run
if: needs.skip_if_release.outputs.should_skip != 'true'
|| needs.should_run.outputs.javascript_changed == 'true'
if: needs.should_run.outputs.javascript_changed == 'true' ||
needs.should_run.outputs.deps_changed == 'true'
runs-on: ubuntu-latest

strategy:
Expand All @@ -119,10 +112,9 @@ jobs:

integration:
needs:
- skip_if_release
- should_run
if: needs.skip_if_release.outputs.should_skip != 'true'
|| needs.should_run.outputs.javascript_changed == 'true'
if: needs.should_run.outputs.javascript_changed == 'true' ||
needs.should_run.outputs.deps_changed == 'true'
runs-on: ubuntu-latest

env:
Expand Down Expand Up @@ -160,10 +152,9 @@ jobs:

versioned-internal:
needs:
- skip_if_release
- should_run
if: needs.skip_if_release.outputs.should_skip != 'true'
|| needs.should_run.outputs.javascript_changed == 'true'
if: needs.should_run.outputs.javascript_changed == 'true' ||
needs.should_run.outputs.deps_changed == 'true'
runs-on: ${{ github.ref == 'refs/heads/main' && vars.NR_RUNNER || 'ubuntu-latest' }}

strategy:
Expand Down Expand Up @@ -211,10 +202,9 @@ jobs:
# There is no coverage for external as that's tracked in their respective repos
versioned-external:
needs:
- skip_if_release
- should_run
if: needs.skip_if_release.outputs.should_skip != 'true'
|| needs.should_run.outputs.javascript_changed == 'true'
if: needs.should_run.outputs.javascript_changed == 'true' ||
needs.should_run.outputs.deps_changed == 'true'
runs-on: ${{ github.ref == 'refs/heads/main' && vars.NR_RUNNER || 'ubuntu-latest' }}

strategy:
Expand Down
48 changes: 45 additions & 3 deletions .github/workflows/compatibility-report.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,17 +54,59 @@ jobs:
path: compatibility.md

# Generate the new PR to update the doc in the repo.
- run: |
Copy link
Member

Choose a reason for hiding this comment

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

why was this removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The create-pull-request action actually does this internally. I discovered it while reviewing the documentation for this PR.

git config user.name $GITHUB_ACTOR
git config user.email gh-actions-${GITHUB_ACTOR}@github.com
- run: |
rm -f status.log
- uses: peter-evans/create-pull-request@6d6857d36972b65feb161a90e484f2984215f83e
with:
token: ${{ secrets.NODE_AGENT_GH_TOKEN || secrets.GITHUB_TOKEN }}
Copy link
Member

Choose a reason for hiding this comment

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

not sure i following why you're checking an or here. shouldn't we always use our bot token which is NODE_AGENT_GH_TOKEN?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As a guard for forks. I'm really not sure what happens if it is run from a fork that doesn't have our secrets.

title: "docs: Updated compatibility report"
commit-message: "docs: Updated compatibility report"
branch: "compatibility-report/auto-update"
delete-branch: true
base: main
labels: "documentation"

docs:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not yet sure this job will do what we actually want, but I can't run it from a branch because this PR is originating from a fork. I think it will just be easier to merge it as-is, and then I can do some manual testing and submit another PR if things are not correct.

Copy link
Member

Choose a reason for hiding this comment

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

that's fair

runs-on: ubuntu-latest
if:
github.event_name == 'push' ||
(github.event.workflow_run && github.event.workflow_run.conclusion == 'success') ||
(github.event_name == 'workflow_dispatch' &&
(inputs.repo_target == 'docs' || inputs.repo_target == 'both'))
env:
DOCS_TARGET: src/content/docs/apm/agents/nodejs-agent/getting-started/compatibility-requirements-nodejs-agent.mdx
steps:
- uses: actions/checkout@v4
with:
path: agent
- uses: actions/checkout@v4
with:
repository: newrelic/docs-website
path: docs
- uses: jaxxstorm/action-install-gh-release@71d17cb091aa850acb2a1a4cf87258d183eb941b
with:
repo: newrelic/newrelic-node-versions
platform: linux
arch: amd64
cache: enable
- run: |
nrversions -v -r agent -R ${DOCS_TARGET} 2>docs-status.log

# Upload generated artifacts for potential debugging purposes.
- uses: actions/upload-artifact@v4
with:
name: docs-status.log
path: docs-status.log

# Generate the new PR to update the doc in the repo.
- run: |
rm -f docs-status.log
- uses: peter-evans/create-pull-request@6d6857d36972b65feb161a90e484f2984215f83e
with:
token: ${{ secrets.NODE_AGENT_GH_TOKEN || secrets.GITHUB_TOKEN }}
title: "docs: Updated Node.js agent compatibility report"
commit-message: "docs: Updated Node.js agent compatibility report"
branch: "nodejs-compatibility-report/auto-update"
delete-branch: true
base: develop
labels: "documentation"
1 change: 0 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,6 @@
"prepare-test": "npm run ssl && npm run docker-env",
"lint": "eslint ./*.{js,mjs} lib test bin examples",
"lint:fix": "eslint --fix, ./*.{js,mjs} lib test bin examples",
"lint:lockfile": "lockfile-lint --path package-lock.json --type npm --allowed-hosts npm --validate-https --validate-integrity",
"public-docs": "jsdoc -c ./jsdoc-conf.jsonc && cp examples/shim/*.png out/",
"publish-docs": "./bin/publish-docs.sh",
"services": "docker compose up -d --wait",
Expand Down