Can you please amend the LTS document accordingly?

Can you please amend the LTS document accordingly?

Hi @mcollina, thanks for taking a look to this proposal.
I've Updated the LTS document as requested, I'm not 100% sure if it has the expected format, I just tried to make it fit.

@RafaelGSS are you ok in supporting this change in case there are issues? I don't expect there will be but better safe than sorry.

@RafaelGSS are you ok in supporting this change in case there are issues? I don't expect there will be but better safe than sorry.

Yes, happy to support it.

If we add some runtime guarantee that is not node, the question would be why we need to against the support of deno or bun?

If N|Solid is bundling Node.js, why it need to be tested seperately?

If we add some runtime guarantee that is not node, the question would be why we need to against the support of deno or bun?

If N|Solid is bundling Node.js, why it need to be tested seperately?

I'd +1 the question, because it's a valid concern

If we add some runtime guarantee that is not node, the question would be why we need to against the support of deno or bun?

My generic take on this is that we can support them if our tests run unmodified.

My generic take on this is that we can support them if our tests run unmodified.

We should write it down somewhere in case.

Anyway my idea is:

do we block a PR if the NSolid CI does not work?

Assuming the answer is "no" I would move this run by:

1. set it as optional by setting continue-on-error

Or:

1. I would move it to a separate workflow
2. I would run the workflow on merge on main (as we do for the package managers: https://github.com/fastify/fastify/blob/main/.github/workflows/package-manager-ci.yml)
3. (idea/suggestion) Add a notification to someone (maybe by opening an issue here or elsewhere)

I think creating a new workflow with continue-on-error should be the most appropriate as it won't require any change in the current setup and any errors in nsolid CI can be easily mitigated by us (nodesource).

I think creating a new workflow with continue-on-error should be the most appropriate as it won't require any change in the current setup and any errors in nsolid CI can be easily mitigated by us (nodesource).

Hi @RafaelGSS what you think if we use the same workflow and put the continue-on-erroronly for nsolid, or any other runtime if it is explicitly set.

something like this:

    continue-on-error: ${{ matrix.continue-on-error || false }}
    strategy:
      matrix:
        runtime: [node, nsolid]
        node-version: [14, 16, 18, 20]
        os: [macos-latest, ubuntu-latest, windows-latest]
        include:
          - runtime: nsolid
            nsolid-version: 5
            continue-on-error: true
        exclude:
        # excludes node 14 on Windows
          - os: windows-latest
            runtime: node
            node-version: 14
        # Nsolid 5 does not support Node.js 14, 16
          - runtime: nsolid
            node-version: 14
          - runtime: nsolid
            node-version: 16

I am not opposed to adding the extra runtime check (though I do wonder why this particular one is necessary given it is a direct rebuild of core). My only concern is regarding the implementation. This PR introduces a lot of complicated configuration to appease the fact that the runtime doesn't support all of the core versions we currently support. I'd rather see a completely separate "alternative runtimes" set of workflows added. Or maybe make the jobs reusable and define new matrix strategies utilizing them.

hi @jsumners, thanks for joining to the discussion, I really appreciate it.
I understand you think the PR introduces a lot of complicated configuration because of the exclusions added for Node14 and Node16, is that right?
N|solid has been releasing versions since Node4(argon) and I can adjust the pipeline to run with N|solid4(Node 14 - Node16) so we can remove those exclusions making it 'compatible' with the supported core version.

I'd rather see a separate workflow that is dedicated to alternative runtimes.

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.