search

getappmap / appmap-agent-js

This project is deprecated. Please use https://github.com/getappmap/appmap-node/ to record Node.js applications.
Other
27 stars 8 forks source link

Move CI from Travis to GitHub Actions #234

Closed dividedmind closed 1 year ago

dividedmind commented 1 year ago

Note for deploy to work NPM_TOKEN needs to be provided as a secret.

dividedmind commented 1 year ago

Can you take another look? Obviously please don't merge as-is, I need to rebase the fixups, I'll take care of it before merging.

Here are my three remarks:

* consider that `npm run coverage` does perform integration tests.

Fixed.

* I would move log changes to another PR.

Thinking about it some more, I think we should keep the tactical changes and open an issue for a more strategic treatment on this. We need to keep moving forward :)

* `fswin` seems heavy and polute the pure path component.

It's the opposite of heavy, it's a dependency-less thin wrapper on the relevant OS APIs. Perhaps it does pollute the path component a bit, but I don't see much of a way around it without needlessly complicating the code. Every abstraction is leaky in the end :)

brikelly commented 1 year ago

@lachrist can you please respond to @dividedmind's comments?

appland-release commented 1 year ago

:tada: This PR is included in version 13.9.2 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

appland-release commented 1 year ago

:tada: This PR is included in version 14.0.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: