search

mathiask88 / node-snap7

node.js wrapper for snap7
MIT License
166 stars 59 forks source link

add GitHub Actions #94

Closed mathiask88 closed 1 year ago

mathiask88 commented 1 year ago

@Apollon77 I tried to be as minimalist as possible. Do you miss any functionality from your PR?

Apollon77 commented 1 year ago

In a first mobile device view looks good. Go ahead

Apollon77 commented 1 year ago

PS: the question is if it not makes sense to add node-gyp as dep to make sure it also works for users with nodes 12/14/16 that most likel never get a new bpm version but python 3.12 will go out next year.

mathiask88 commented 1 year ago

Python version is just for latest macos. They dropped support of disutils in python 3.12. prebuild bundles it's own node-gyp. I would keep this workaround until prebuild upgrades to node-gyp 10 that solves the issue. Instead of forcing python 3.11 I could also install disutils via pip in de CI. Node 12/14/16 should build fine. But they are out of maintenance, so good if it works, but won't put too much effort to get these old versions working.

Apollon77 commented 1 year ago

Sure ... yes with prebuild ideally the issue is also "solved" for the users ... yes

Apollon77 commented 1 year ago

@mathiask88 Please check ... 1.0.7 is not available on npm!

mathiask88 commented 1 year ago

I know, still fighting with the CI and old node versions. Will apply a patch this evening or latest on Saturday. And then release the new version to npm.

Apollon77 commented 1 year ago

@mathiask88 did you had any chance?

mathiask88 commented 1 year ago

released on npm.. Sorry I had to deal with the npm/Github service this week (which was very nice and helpful btw) because I never setup 2FA and my account got locked 😅

Apollon77 commented 1 year ago

Thank you very much!