Skip to content
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

nestjs-core: extend test-all-versions cover #2197

Open
blumamir opened this issue May 11, 2024 · 3 comments
Open

nestjs-core: extend test-all-versions cover #2197

blumamir opened this issue May 11, 2024 · 3 comments
Assignees
Labels
never-stale pkg:instrumentation-nestjs-core size/S Denotes a PR that changes 10-29 lines, ignoring generated files. up-for-grabs Good for taking. Extra help will be provided by maintainers

Comments

@blumamir
Copy link
Member

The @nestjs/core instrumentation defines an open range for supported versions >=4.0.0, which is capped in #2196 to the latest current major version.

However, since the instrumentation was last updated, the package released few more major versions, which the instrumentation will attempt to patch, are not part of the current .tav.yaml range

"@nestjs/core":
  - versions: "^8.2.0 || 8.1.2 || 8.1.1 || 8.0.11 || 8.0.9 || 8.0.6"
    peerDependencies:
        - "@nestjs/common@^8.0.0"
        - "@nestjs/microservices@^8.0.0"
        - "@nestjs/platform-express@^8.0.0"
        - "@nestjs/websockets@^8.0.0"
        - "rxjs@^7.1.0"
    commands: npm run test

  - versions: "^7.6.17 || 7.6.15 || 7.6.13 || 7.6.12"
    peerDependencies:
        - "@nestjs/common@^7.0.0"
        - "@nestjs/microservices@^7.0.0"
        - "@nestjs/platform-express@^7.0.0"
        - "@nestjs/websockets@^7.0.0"
        - "rxjs@^6.0.0"
    commands: npm run test

  - versions: "6.11.11"
    peerDependencies:
        - "@nestjs/common@^6.0.0"
        - "@nestjs/microservices@^6.0.0"
        - "@nestjs/platform-express@^6.0.0"
        - "@nestjs/websockets@^6.0.0"
        - "rxjs@^6.0.0"
    commands: npm run test

This issue is to add those versions to the test-all-versions yaml, make sure instrumentation works for these versions (exaime any potential changes to the functions we patch with new versions) and consider upgrading the version in devDependencies.

@blumamir blumamir added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. up-for-grabs Good for taking. Extra help will be provided by maintainers never-stale pkg:instrumentation-nestjs-core labels May 11, 2024
@trentm trentm changed the title nestjs-core: extend tes-all-versions cover nestjs-core: extend test-all-versions cover May 13, 2024
@trentm
Copy link
Contributor

trentm commented May 13, 2024

That last 7.x release was: '7.6.17': '2021-05-18T08:43:02.672Z',
So perhaps its test-all-versions (TAV) config could be limited to just the latest "7.6.17".

The last 8.x release was: '8.4.7': '2022-06-14T07:25:26.179Z',
So, perhaps limit this as well to just the latest.

The last 9.x was: '9.4.3': '2023-06-12T10:25:05.603Z',
Again, perhaps limit to just testing the latest 9.x.

Then perhaps add a range of 10.x releases.
Now that we are on test-all-versions@6.1.0 we could use use the versions mode feature (https://github.com/watson/test-all-versions?tab=readme-ov-file#advanced-versions-usage) to limit to just testing a few. E.g. something like:

  versions:
    include: ^10.0.0
    mode: max-5

@amkarn258
Copy link

Hi @trentm ,

Can you please assign this issue to me? I'm new to opensource and would like to help.

@trentm
Copy link
Contributor

trentm commented Jun 19, 2024

@amkarn258 Done. Thanks for taking a look!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
never-stale pkg:instrumentation-nestjs-core size/S Denotes a PR that changes 10-29 lines, ignoring generated files. up-for-grabs Good for taking. Extra help will be provided by maintainers
Projects
None yet
Development

No branches or pull requests

3 participants