Skip to content

fix: Add upload artifact to on-pull-request to save PR number for later auto-merge #178

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

Closed
wants to merge 1 commit into from

Conversation

dreamorosi
Copy link
Contributor

Description of your changes

PR #169 introduced a workflow to auto-merge PRs from Dependabot which didn't work. The new workflow depended on the existence of an artefact created & saved from the previous workflow.

This is the recommended way to share data between workflows/jobs and uses a mechanism of upload & download artefact files between different workflows/jobs. In the previous PR I introduced the download part but since there was no upload, and the workflow depended on the existence of an artefact to find the PR number, the auto-merge job failed.

This PR introduces a new job/step in the on-pull-request workflow that should be executed only when the actor is dependabot and that should create a file with the PR number and upload it as an artefact. This artefact would then be used by the auto-merge workflow to merge.

How to verify this change

If the change is effective, re-running the on-pull-request workflow for an existing PR opened by dependabot should cause the PR to be merged. Same applies to a new dependabot PR.

Related issues, RFCs

#169
#126

PR status

Is this ready for review?: YES
Is it a breaking change?: NO

Checklist

  • My changes meet the tenets criteria
  • I have performed a self-review of my own code
  • I have commented my code where necessary, particularly in areas that should be flagged with a TODO, or hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • The code coverage hasn't decreased
  • I have added tests that prove my change is effective and works
  • New and existing unit tests pass locally and in Github Actions
  • Any dependent changes have been merged and published in downstream module
  • The PR title follows the conventional commit semantics

Breaking change checklist

  • I have documented the migration process
  • I have added, implemented necessary warnings (if it can live side by side)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@dreamorosi dreamorosi added the automation This item relates to automation label Aug 13, 2021
@dreamorosi dreamorosi self-assigned this Aug 13, 2021
@github-actions
Copy link
Contributor

Coverage after merging fix/dependabot-auto-merge into main

100.00%

Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
./packages/logger/src
   Logger.ts100%100%100%100%
   helpers.ts100%100%100%100%
   index.ts100%100%100%100%
./packages/logger/src/config
   ConfigService.ts100%100%100%100%
   EnvironmentVariablesService.ts100%100%100%100%
   index.ts100%100%100%100%
./packages/logger/src/formatter
   LogFormatter.ts100%100%100%100%
   PowertoolLogFormatter.ts100%100%100%100%
   index.ts100%100%100%100%
./packages/logger/src/log
   LogItem.ts100%100%100%100%
   index.ts100%100%100%100%
./packages/metrics/src
   Metrics.ts100%100%100%100%
   index.ts100%100%100%100%
./packages/metrics/src/config
   ConfigService.ts100%100%100%100%
   EnvironmentVariablesService.ts100%100%100%100%
   index.ts100%100%100%100%

@dreamorosi
Copy link
Contributor Author

Also apologies for the noise, testing these workflows locally is tricky.

@dreamorosi
Copy link
Contributor Author

Closing this PR & deleting branch in favor of projen based one.

@dreamorosi dreamorosi closed this Dec 1, 2021
@dreamorosi dreamorosi deleted the fix/dependabot-auto-merge branch December 1, 2021 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automation This item relates to automation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants