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

Support spy.on with nullish prototype #118

Merged
merged 4 commits into from Apr 6, 2023

Conversation

sebamarynissen
Copy link
Contributor

Fixes #117.

Copy link
Member

@keithamus keithamus left a comment

Choose a reason for hiding this comment

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

LGTM

@keithamus
Copy link
Member

Looks like travis CI is no longer working. I'll find some time to port to GitHub Actions and then get this merged, it might be a while though.

@sebamarynissen
Copy link
Contributor Author

I can have a look at writing a GitHub action for it if you want, at least for running the tests on every PR. I don't have any experience with writing an action for publishing to npm, but I'll see what I can do. This should happen for every commit on master right? Is the version increased manually or should this happen in an action as well?

@keithamus
Copy link
Member

I'd like to move Chai packages to a model I use at work, which is that we have a build step that simply runs npm it (e.g. here: https://github.com/github/relative-time-element/blob/main/.github/workflows/test.yml) and an action which runs whenever we publish a release via the GitHub UI, which publishes the package to the various registries (e.g. here: https://github.com/github/relative-time-element/blob/main/.github/workflows/publish.yml).

If you'd like to try and add those in a PR that'd be a great contribution that would be much appreciated!

@sebamarynissen
Copy link
Contributor Author

@keithamus I created another PR that sets up GitHub actions for CI and publishing (#119). The PR was created off the master branch, so it does not include the changes from this PR yet. I propose to merge #119 first, and then we can merge this one, perhaps after a rebase.

@keithamus keithamus merged commit 419cddd into chaijs:main Apr 6, 2023
5 checks passed
@sebamarynissen
Copy link
Contributor Author

@keithamus Is there anything that prevents releasing this on npm? I'm migrating a large codebase to Vue 3 at the moment and I'm using my own fork in package.json for it now

{
  "dependencies": {
    "chai-spies": "github:sebamarynissen/chai-spies#feature/nullish-prototype",
  }
}

but it obviously would be nicer to be able to just use chai-spies from npm.

Let me know if there's anything I can do to help if needed!

@keithamus
Copy link
Member

keithamus commented Oct 31, 2023

@sebamarynissen v1.1.0 has been released. Thanks!

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

Successfully merging this pull request may close these issues.

spy.on doesn't work with nullish prototype
2 participants