Skip to content

chore: add new PR workflow #2866

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

Draft
wants to merge 26 commits into
base: main
Choose a base branch
from

Conversation

david-luna
Copy link
Contributor

@david-luna david-luna commented May 29, 2025

Which problem is this PR solving?

Changes the PR workflow to a single file to improve it making it faster with less resource consumption. The goal is to provide feedback to the PR author by compiling, testing and reporting coverage only for the affected packages in the PR (as for now it does a compilation, test, and coverage report for all in nodejs v18). More details on the plan can be found in #2870

Closes: #2870

Short description of the changes

  • add new PR workflow with:
    • single compile step for affected packages. results are cached
    • unit and TAV test for affected packages for a matrix of nodejs versions. Reusing compilation results from cache. coverage reports are cached
    • get coverage reports from all nodejs versions (in cache) and merge them. then upload them to codecov
  • add scripts to support such workflow
    • *:ci:affected script at root to execute tasks only for affected packages (leveraging nx CLI)
  • update codecov.yml to assign flags to each package so reports can be updated independently

Checklist

Codecov flags feature require a first manual upload to create them so it would be helpful to resume #2214 to at least have a script at the root to testa and upload all reports. There is also some PRs that affect the flag configuration so it would be preferable to have them merged 1st

Copy link
Contributor

This package does not have an assigned component owner and is considered unmaintained. As such this package is in feature-freeze and this PR will be closed with 14 days unless a new owner or a sponsor (a member of @open-telemetry/javascript-approvers) for the feature is found. It is the responsibility of the author to find a sponsor for this feature.
Are you familiar with this package? Consider becoming a component owner.

@pichlermarc pichlermarc added has:sponsor This package or feature has a sponsor that has volunteered to review PRs and respond to questions and removed pkg-status:unmaintained:autoclose-scheduled labels Jun 13, 2025
- name: Install
run: npm ci
- name: Download Build Artifacts
# TODO: add the rest of versions (18.19.0 & 20.6.0)? or are these enough??
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if easy to do we could also use these for completeness, but just using 18 and 20 is likely more than okay 🙂

@@ -34,7 +35,7 @@ jobs:
if-no-files-found: error
retention-days: 1

unit-test:
unit-and-tav-test:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keeping the name unit-test would help with repo settings, since unit-test (18) and unit-test (18.19.0) required checks. Repo managment is now automated from another repo, so changing the check's name would require us to open a PR in that other repo before merging this (which would then block other PRs from getting merged)

If we want to change the name we can do so in a smaller follow-up PR. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Discuss] Improvements in CI pipeline for PRs