search

mattdavis90 / node-tado-client

A Tado API client for Node
MIT License
47 stars 18 forks source link

ci: add main workflow #60

Closed abn closed 1 month ago

abn commented 1 month ago

Adding a simple workflow to perform lint checks, build checks and ensure package can publish. Rolled everything into one as separating them seemed overkill for the project for now.

abn commented 1 month ago

@mattdavis90 is it intentional that you are storing build output in the repository? The publishing (dry-run) step is failing because pnpm build generates a build output that differs from what is in the repository.

abn commented 1 month ago

I have created #61 making the assumption that build is not desired in the source tree. If that is merged this can simply be rebased to trigger the runs.

mattdavis90 commented 1 month ago

The output is deliberate maintained to keep compatibility with JavaScript only users. The library was originally JS only and the primary user which is my NodeRed integration is also JS only so those files will need to remain. Thanks

abn commented 1 month ago

@mattdavis90 ack, i will add a check to ensure they are regenerated.

Out of interest (since I am not a JS/TS dev), wouldn't these be better served perhaps as a release artifact as part of the GH release?

mattdavis90 commented 1 month ago

Possibly. They need to make their way into the NPM bundle. I guess they probably don't need to be in the repo but they previously were and I wanted to maintain backwards compatibility

abn commented 1 month ago

Updated with the following:

  1. Verify that files do not change after a build.
  2. Ensure build script includes a format.
mattdavis90 commented 1 month ago

Hi, I've merged the two branches that added/updated features. They now conflict with this branch. I'm thinking more about the build artefacts in the repo and think maybe you're right and we remove them. Happy to do that in this PR, without opening another.

abn commented 1 month ago

Yeah the merge conflicts are not fun :).

Do you prefer to remove it here and then attach it to release?

In order to not break any existing users, how are these referred to presently? Raw url?

mattdavis90 commented 1 month ago

It was the merge conflict last night that made me thing again about having the build results in the source tree.

I think the chances of anyone using a direct Git URL in their package.json should be pretty small and if they are then hopefully they'll understand why it's been removed from the source tree.

I think we removed the build directory as per #61 and then on release build bundle up the artefacts into the NPM package like before. pnpm release should handle that as long as the artefacts are present.

abn commented 1 month ago

Done. Removed the build directory. We can modify the release artifacts as we need in #62.

mattdavis90 commented 1 month ago

Looks great. Thank you so much!!

I'll merge this now, then prep for a release under #62