chore(build): broke up pr workflow & measure package size #1031

dreamorosi · 2022-07-29T17:55:05Z

Description of your changes

This PR proposes changes to the workflow that is run every time a new PR is created & synchronised (aka new commit is pushed). It also spins off in a separate file the workflow that measures the size of the packages as well as changing is trigger from even based to on-demand.

On PR code update

The workflow that runs every time new code is added to a PR was running completely in sequential mode before this PR, aside from the matrix feature that was used to select the runtime. I have applied the same pattern also to the examples and break up the two sections in two different jobs that can now run in parallel:

I also have removed completely all the lerna-related commands in favor of the npm workspaces related ones. This allows us to select the packages we want to run the commands in.

Initially I used a more terse version:

npm run lint -ws

but then I moved to a slightly longer, but more explicit one, that will allow us to exclude packages that are work in progress until they are ready, while still being part of the workspace:

npm run lint -w packages/logger -w packages/tracer -w packages/metrics

Each job is also now caching the content of the node_modules folders (both main and ones in examples/*) between different executions. The cache uses a combination of environment variables, node version, and hash of the package-lock.json as key so that if environment, version, or dependencies change the cache is rebuilt.

Here's an example of cache hit.

The current time for all the checks to run is now ~2.5 minutes when all caches are hit 💨 🥵 ⚡ (around ~3min when no cache)

Measure packages size

This action was previously running after the one described in the section above. This meant each new commit was causing the action to run and leave a comment. After several weeks some of us (me included) have felt like this was generating a bit too much noise in the comments section.

This PR moves the action in its own workflow and allows maintainers to run it on demand by specifying a PR number. The action will use the PR number to get the info related to head and base and then, after having computed the changes and sizes, it will leave a comment only when manually triggered.

This PR is dependant on this other PR made in the action's repo.

Like in the previous workflow, I have also removed the lerna related commands and used the npm workspaces ones. This has the added benefit of avoiding running unnecessary commands in the examples/* folders.

Finally, this new version of the action and workflow, once merged will allow to safely run the checks also for PRs that come from external contributors.

Additionally, both workflows are now `lerna`-free and use `npm workspaces` 🤟

How to verify this change

See example of the On PR code update workflow run in a fork here:
https://github.com/dreamorosi/aws-lambda-powertools-typescript/actions/runs/2724186137

Also see an example of the comment left on a PR opened by an external user on a fork (of a fork):
https://github.com/dreamorosi/aws-lambda-powertools-typescript/pull/3

Related issues, RFCs

Issue number: #991

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
My changes generate no new warnings
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

N/A

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

dreamorosi · 2022-07-29T18:03:45Z

examples/sam/package.json

@@ -18,6 +18,7 @@
  },
  "devDependencies": {
    "@types/aws-lambda": "^8.10.86",
+    "@types/jest": "^27.5.2",


This change is needed because we are now running the tests outside of the lerna scope. For this reason the dependency (which is present in both the examples/cdk and the main package) is also needed here now.

.github/workflows/measure-packages-size.yml

ijemmy

Got a question around environment variable for cache key.

Apart from that, everything looks good to me.

.github/workflows/pr_lint_and_test.yml

ijemmy

LGTM

* Add a cdk app capable of building and publishing Lambda Layer * add github action for e2e * add local layer construct npm package * disable fail fast * fix RUNTIME naming in matrix * fix tests * fix outputs export name * Add capability to pass package version to layer builder/publisher * Add initial doc * Add github workflows * add gitignore * fix install deps * pass layer name * make layer public and store details in ssm * fix e2e tests * fix context * remvove groups for tests * publish layer on release * fix account in doc * fix test deps * deploy to all region * fix account number for layer * fix unit tests * add install for layer deps in pr workflow * run unit test of layer publisher for supported node versions only * fix node version in doc * add node 16 to layer e2e * fix comments * fix doc * rename layer to TypeScript * take care of comments * remove layer from doc for now * remove pasted commenbt * chore(build): broke up pr workflow & measure package size (#1031) * chore: broke up pr workflow & measure package size * chore: explicitly set packages where to run cmds * fix: added missing dependency to examples/sam * added cache in workflow * chore: updated workflow + removed env var from command * chore: updated action version * fix: removed redundant env variable Co-authored-by: Florian Chazal <chazalf@amazon.com> Co-authored-by: Andrea Amorosi <dreamorosi@gmail.com>

dreamorosi added 2 commits July 29, 2022 19:35

chore: broke up pr workflow & measure package size

1585596

chore: explicitly set packages where to run cmds

0db209f

dreamorosi self-assigned this Jul 29, 2022

github-actions bot added the internal PRs that introduce changes in governance, tech debt and chores (linting setup, baseline, etc.) label Jul 29, 2022

fix: added missing dependency to examples/sam

75df9a3

dreamorosi added the automation This item relates to automation label Jul 29, 2022

dreamorosi commented Jul 29, 2022

View reviewed changes

.github/workflows/measure-packages-size.yml Outdated Show resolved Hide resolved

dreamorosi linked an issue Jul 29, 2022 that may be closed by this pull request

Maintenance: package size report failing #991

Closed

dreamorosi added 3 commits July 31, 2022 19:36

added cache in workflow

736dbf6

chore: updated workflow + removed env var from command

b108cfd

chore: updated action version

017c999

dreamorosi marked this pull request as ready for review August 4, 2022 08:46

dreamorosi requested review from flochaz, saragerion and ijemmy and removed request for saragerion August 4, 2022 08:47

flochaz previously approved these changes Aug 4, 2022

View reviewed changes

.github/workflows/measure-packages-size.yml Outdated Show resolved Hide resolved

ijemmy previously approved these changes Aug 4, 2022

View reviewed changes

.github/workflows/pr_lint_and_test.yml Outdated Show resolved Hide resolved

.github/workflows/pr_lint_and_test.yml Outdated Show resolved Hide resolved

fix: removed redundant env variable

8e303d9

dreamorosi dismissed stale reviews from ijemmy and flochaz via 8e303d9 August 4, 2022 10:45

dreamorosi requested review from flochaz and ijemmy August 4, 2022 10:46

ijemmy approved these changes Aug 4, 2022

View reviewed changes

flochaz approved these changes Aug 9, 2022

View reviewed changes

dreamorosi merged commit 4a1c436 into main Aug 9, 2022

dreamorosi deleted the chore/update_pr_lint_and_test_workflow branch August 9, 2022 12:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(build): broke up pr workflow & measure package size #1031

chore(build): broke up pr workflow & measure package size #1031

dreamorosi commented Jul 29, 2022 •

edited

Loading

dreamorosi Jul 29, 2022

ijemmy left a comment

ijemmy left a comment

chore(build): broke up pr workflow & measure package size #1031

chore(build): broke up pr workflow & measure package size #1031

Conversation

dreamorosi commented Jul 29, 2022 • edited Loading

Description of your changes

On PR code update

The current time for all the checks to run is now ~2.5 minutes when all caches are hit 💨 🥵 ⚡ (around ~3min when no cache)

Measure packages size

Additionally, both workflows are now lerna-free and use npm workspaces 🤟

How to verify this change

Related issues, RFCs

PR status

Checklist

Breaking change checklist

dreamorosi Jul 29, 2022

Choose a reason for hiding this comment

ijemmy left a comment

Choose a reason for hiding this comment

ijemmy left a comment

Choose a reason for hiding this comment

dreamorosi commented Jul 29, 2022 •

edited

Loading

Additionally, both workflows are now `lerna`-free and use `npm workspaces` 🤟