feat: added a shared workflow to auto update browserslist in the MFEs #29
Conversation
| commit-message: 'chore: update browserslist' | ||
| title: Update browserslist DB | ||
| body: | | ||
| Updated browserslist DB |
There was a problem hiding this comment.
I would provide a much more verbose body. Is there a SLA to merging that teams should have? What is the importance of the change? Could we do a short summary of the change? (you can echo things to the Actions environment then use them later in the file, for example this line:
echos variable $PROJECT_ID to the local $GITHUB_ENV, and then it's used in the next step:
There was a problem hiding this comment.
Is there a SLA to merging that teams should have?
I believe we could auto-merge the resulting PR from this workflow. The resulting changes from browserslist --update-db command update a tiny portion of the package-lock.json file, and is pretty low risk to auto-merge, assuming CI and other status checks pass.
That way, consuming teams won't need to triage/prioritize merging the resulting PR.
| commit-message: 'chore: update browserslist' | ||
| title: Update browserslist DB | ||
| body: | | ||
| Updated browserslist DB |
There was a problem hiding this comment.
Is there a SLA to merging that teams should have?
I believe we could auto-merge the resulting PR from this workflow. The resulting changes from browserslist --update-db command update a tiny portion of the package-lock.json file, and is pretty low risk to auto-merge, assuming CI and other status checks pass.
That way, consuming teams won't need to triage/prioritize merging the resulting PR.
| @@ -0,0 +1,7 @@ | |||
| { | |||
| "name": "Auto Update Browserslist Workflow", | |||
There was a problem hiding this comment.
nit: "Auto Update Browserslist DB Workflow" maybe?
There was a problem hiding this comment.
Similarly, should the file name also include DB, e.g. update-browserslist-db.properties.json?
| run: npm ci | ||
|
|
||
| - name: Update dependencies | ||
| run: npx browserslist@latest --update-db |
There was a problem hiding this comment.
Note: I just stumbled on https://github.com/c2corg/browserslist-update-action, which is a Github Action we might be able to use here instead. Worth noting that the README for that action mentions:
v1 uses command
npx browserslist@latest --update-dbwhich is now deprecated and to be removed in a future major browserslist release.
It is being replaced with a standalone NPM package update-browserslist-db instead.
|
@abdullahwaheed, sprint check: will you have time to address the reviews in the next couple of weeks? |
|
label: waiting on author |
arbrandes
added
the
waiting on author
…-request-automerge
|
@adamstankiewicz I have used this action to enable Auto Merge functionality and made suggested changes. Please note that these conditions must met to get PR auto merge. |
|
remove label: waiting on author |
abdullahwaheed
requested review from
adamstankiewicz and
sarina
and removed request for
adamstankiewicz and
sarina
abdullahwaheed
requested review from
arbrandes and
adamstankiewicz
and removed request for
arbrandes
|
I see you've requested me for a review. From an Actions perspective this seems fine but I don't know anything about what it's actually doing and am not going to test, so I will not be a reviewer. |
|
I approve, but I'm not sure I should be able to merge this. I'll ask around and circle back. |
|
This also looks good to me! ✅ |
arbrandes
changed the title
|
@adamstankiewicz @arbrandes to enable auto merge functionality, |
|
@abdullahwaheed, I think we should allow repo owners/maintainers to decide. Maybe as part of a checklist on the PR description, checking with the reviewer to agree auto-merging should be enabled for that repo. |
|
sure, i'll add appropriate comments in the PR description, and i'll also update this shared workflow to make auto merging |
Added a shared Workflow to auto update browserslist