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

Integrating @matrixai/events into ESM 2.x #32

Merged
merged 4 commits into from
May 14, 2024
Merged

Conversation

CMCDragonkai
Copy link
Member

@CMCDragonkai CMCDragonkai commented Sep 3, 2023

Description

The ESM branch needs events and keep up with the 1.9.x.

This currently doesn't yet work because js-events isn't ESM native yet. So have to wait until that is the case.

This branch must be kept up to date with the branch feature-events. https://github.com/MatrixAI/js-async-init/tree/feature-events

Tasks

  • 1. Update the @matrixai/events to the ESM-native version when available
  • 2. Run tests again to see if everything works

Final checklist

  • Domain specific tests
  • Full tests
  • Updated inline-comment documentation
  • Lint fixed
  • Squash and rebased
  • Sanity check the final build

@ghost
Copy link

ghost commented Sep 3, 2023

👇 Click on the image for a new way to code review

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map legend

@CMCDragonkai
Copy link
Member Author

The feature-events branch is now on 1.10.0. So it's evolved quite a bit. The 2.x releases will need a major update to bring all the changes.

@CMCDragonkai
Copy link
Member Author

All the commits here https://github.com/MatrixAI/js-async-init/commits/feature-events will need to be redone on staging with ESM changes so that way the history looks right.

@CMCDragonkai
Copy link
Member Author

When migrating to ESM, we might as well change over to #22 too.

@amydevs
Copy link

amydevs commented Sep 25, 2023

some notes regarding windows failing CI/CD due to chocolatey v2.0

The windows before_build script should have this include so that refreshenv will resolve to the powershell alias:

before_script:
    - mkdir -Force "$CI_PROJECT_DIR/tmp"
    # so that refreshenv will resolve to the correct Powershell alias
    - Import-Module $env:ChocolateyInstall\helpers\chocolateyProfile.psm1

choco-install.ps1 should have the package name changed to nodejs instead of nodejs.install. As the former is a dependent of the latter, installing a newer version of the latter directly will make chocolatey complain that it will conflict with dependents.

# Install nodejs v20.5.1 (will use cache if exists)
$nodejs = "nodejs"
choco install "$nodejs" --version="20.5.1" --require-checksums -y

related: chocolatey/choco#3176

@CMCDragonkai
Copy link
Member Author

Yes that needs to be applied across the repos otherwise CI on windows will start failing.

@CMCDragonkai
Copy link
Member Author

So the feature-events is maintaining the older commonjs version. While staging is currently on ESM.

This PR needs to merge into staging and release another 2.x version in order to keep up to date with the 1.x version that is being branched off in feature-events.

@CMCDragonkai CMCDragonkai self-assigned this May 14, 2024
@CMCDragonkai
Copy link
Member Author

some notes regarding windows failing CI/CD due to chocolatey v2.0

The windows before_build script should have this include so that refreshenv will resolve to the powershell alias:

before_script:
    - mkdir -Force "$CI_PROJECT_DIR/tmp"
    # so that refreshenv will resolve to the correct Powershell alias
    - Import-Module $env:ChocolateyInstall\helpers\chocolateyProfile.psm1

choco-install.ps1 should have the package name changed to nodejs instead of nodejs.install. As the former is a dependent of the latter, installing a newer version of the latter directly will make chocolatey complain that it will conflict with dependents.

# Install nodejs v20.5.1 (will use cache if exists)
$nodejs = "nodejs"
choco install "$nodejs" --version="20.5.1" --require-checksums -y

related: chocolatey/choco#3176

I've just done this in MatrixAI/js-events@849bc20

@CMCDragonkai
Copy link
Member Author

Actually it might not work anymore. We might need to also change our windows build tags to:

  tags:
    - saas-windows-medium-amd64

At the time I saw the pipeline run, no windows runners were available. This may be a temporary change until we move to GitHub actions.

@CMCDragonkai
Copy link
Member Author

Yep now it works: MatrixAI/js-events@9de0ee5 - this needs to be done for all repos @tegefaulkes @amydevs.

@CMCDragonkai
Copy link
Member Author

Yep now it works: MatrixAI/js-events@9de0ee5 - this needs to be done for all repos @tegefaulkes @amydevs.

@aryanjassal how fast are we changing to github so we don't have to do this?

@CMCDragonkai
Copy link
Member Author

Merging, this will be released as 2.1.0 in staging.

@CMCDragonkai CMCDragonkai merged commit 7ddebde into staging May 14, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants