Maintenance: Improve CI for PRs about aws-lambda-powertools-typescript HOT 13 CLOSED

@alan-churley so it seems that my statement about Dependabot PRs was wrong and even though the results of checks didn't appear under the "Conversation" tab the the workflow still ran:

I think they didn't appear in the overview because I was looking at a closed PR, in the PR that was opened today (while still open)

A new PR #124 opened today had both on-push and on-pull-request workflows run.

If we want to avoid both running we could consider changing the on-push.yml to something like:

on:
  push:
    branches-ignore:
      - 'dependabot/**'

from aws-lambda-powertools-typescript.

The action used to setup node actions/setup-node@v2 recently released support for package caching but at this time it only works with package-lock.json and not npm-shrinkwrap.json which we are using.

There's an open issue on the action's repo saying that support for npm-shrinkwrap.json should be released in the coming months.

When this will be released we'll be able to add a key like this:

- uses: actions/setup-node@v2
  with:
    node-version: '14'
    cache: npm

Alternatively we could move back to package-lock.json but not sure it's worth it.

@saragerion thoughts?

from aws-lambda-powertools-typescript.

I don't fully understand the ask here, the tests are already run against each PR automatically, and the results are shown on the PR page?

For example,

You can see here that the chore has passed all tests, and the feat:add tracer has not

If you go into the PR you can see more details, here the on-push-event runs all of our test suite

And if you click on details, in this example https://github.com/awslabs/aws-lambda-powertools-typescript/pull/107/checks?check_run_id=2973962167 you can see that this PR has for example failed linting

from aws-lambda-powertools-typescript.

Correct and apologies for not explaining it better, I was referring to PRs opened by Dependabot, like this one, that don't originate from a push but from a pull_request event.

It could be as simple as adding an event at after L3 of the current .workflows/on-push.yml file or having a dedicated workflow only for PRs, which I am open for discussion.

from aws-lambda-powertools-typescript.

Ah I understand, thanks for explaining.

I think a separate workflow would be better personally, as we may want to do more on PR than on every push?

We will have to add some logic for dependabot PRs as they won't be able to push coverage reports for example, which we would still want for normal PRs. (https://github.blog/changelog/2021-02-19-github-actions-workflows-triggered-by-dependabot-prs-will-run-with-read-only-permissions/)

from aws-lambda-powertools-typescript.

⚠️ COMMENT VISIBILITY WARNING ⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

from aws-lambda-powertools-typescript.

Personally I prefer having a separate on PR and on Push workflow, as it gives us more possibility going forward.

I think we need to fix the logic that prevents the On Push workflow running on PR as we currently stand as most of the logic is the same?

from aws-lambda-powertools-typescript.

Totally agree that they should be separate, and currently they are as one runs on: pull_request while the other on: push.

The problem is that my original assumption about Dependabot was wrong and what happens is that the bot pushes and opens the PR I rapid succession which causes the two workflows to run in the same batch/check.

I've been investigating but so far the easiest way to avoid this is to ignore all branches that start with dependabot/* in the push workflow.

@alan-churley what do you think?

from aws-lambda-powertools-typescript.

from aws-lambda-powertools-typescript.

@dreamorosi what I am actually suggesting is that we don't want the On Push to run on any PRs, not just the Dependant, as they are essentially running the same tests

from aws-lambda-powertools-typescript.

@alan-churley I see and I misunderstood the way these events/workflow work. As you suggested in our conversation offline we need to look into it more & it's not the end of the world if both run.

We might also explore what the Python repo is doing and/or look into Mergify as Heitor suggests.

from aws-lambda-powertools-typescript.

Going to close this issue as CI for PRs has already been addressed, Dependabot's PR can be addressed with a separate issue at a later time.

from aws-lambda-powertools-typescript.

⚠️ COMMENT VISIBILITY WARNING ⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

from aws-lambda-powertools-typescript.