-
Notifications
You must be signed in to change notification settings - Fork 399
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
chore: Added updating of docs site with compat table #2205
Conversation
ca0b058
to
d818987
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2205 +/- ##
==========================================
- Coverage 95.22% 95.22% -0.01%
==========================================
Files 279 279
Lines 44915 44917 +2
==========================================
+ Hits 42769 42770 +1
- Misses 2146 2147 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
54e7239
to
b4b424a
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.
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).
title: "docs: Updated compatibility report" | ||
commit-message: "docs: Updated compatibility report" | ||
branch: "compatibility-report/auto-update" | ||
delete-branch: true | ||
base: main | ||
labels: "documentation" | ||
|
||
docs: |
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'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.
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.
that's fair
steps: | ||
- uses: actions/checkout@v4 | ||
- uses: dorny/paths-filter@v3 | ||
id: filter | ||
with: | ||
filters: | | ||
javascript: | ||
- '**/*.js' |
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.
verify nice
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.
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?
@@ -54,17 +54,59 @@ jobs: | |||
path: compatibility.md | |||
|
|||
# Generate the new PR to update the doc in the repo. | |||
- run: | |
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 was this removed?
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 create-pull-request
action actually does this internally. I discovered it while reviewing the documentation for this PR.
- run: | | ||
rm -f status.log | ||
- uses: peter-evans/create-pull-request@6d6857d36972b65feb161a90e484f2984215f83e | ||
with: | ||
token: ${{ secrets.NODE_AGENT_GH_TOKEN || secrets.GITHUB_TOKEN }} |
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.
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
?
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.
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: |
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.
that's fair
4e9a426
to
7a566f0
Compare
7a566f0
to
6a0d0c5
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.
I can't test this but LGTM
This PR should resolve #2157.