publiclab / spectral-workbench.js

The JavaScript heart of Spectral Workbench; a Public Lab project to record, manipulate, and analyze spectrometric data.
https://spectralworkbench.org
GNU General Public License v3.0
46 stars 35 forks source link

Optimization of averaging code; set up & test the SW JS library #290

Closed Georjane closed 2 years ago

Georjane commented 2 years ago

Make sure these boxes are checked before your pull request is ready to be reviewed and merged. Thanks!

Please be sure you've reviewed our contribution guidelines at https://publiclab.org/wiki/contributing-to-public-lab-software

Please alert developers on plots-dev@googlegroups.com when your request is ready or if you need assistance.

Thanks!

welcome[bot] commented 2 years ago

Thanks for opening this pull request! Dangerbot will test out your code and reply in a bit with some pointers and requests. There may be some errors, but don't worry! We're here to help! 👍🎉😄

gitpod-io[bot] commented 2 years ago

jywarren commented 2 years ago

@Georjane that looks perfect. Now you need to a) change the version number in package.json, and b) run npm install to generate a new package-lock.json file. Then commit those files to the same PR!

jywarren commented 2 years ago

Once we do that all and merge it, we can run the steps to publish to the NPM registry, which would be npm publish -- however you have to have permissions to do that. @Georjane can you create an npmjs.org username if you haven't already and tell me so I can add you to the list of people who can publish? Thanks!

jywarren commented 2 years ago

And just checking, sometimes it's a convention to merge to a stable branch and publish from there, but it looks like in this repository we have just been publishing from the main branch so we should be OK to do that after merging this PR. Let's do be sure we've compiled the code into the /dist/ folders though... the README should say but I think it's either npm run build or grunt build.

jywarren commented 2 years ago

Typically npm modules can just be left uncompiled if they're going to be used by another node-based environment. But since ours is designed to run in the browser, which can't do things like require other JS files using require('modulename'), we have to compile it using Browserify or something similar like Webpack. I forget which we're using here, but it's all taken care of in the build script!

Georjane commented 2 years ago

npmjs.org

Once we do that all and merge it, we can run the steps to publish to the NPM registry, which would be npm publish -- however you have to have permissions to do that. @Georjane can you create an npmjs.org username if you haven't already and tell me so I can add you to the list of people who can publish? Thanks!

Hi @jywarren https://www.npmjs.com/~georjane this is my npm profile which I created with username georjane

Georjane commented 2 years ago

@jywarren @Tlazypanda I made the above changes required. Please can this PR be reviewed again? Thank you

jywarren commented 2 years ago

This looks great. Let's merge and then I'll add you, then you should be able to check out your local main branch, run git pull origin main and get the latest main branch, including this PR.

Then you should be able to run npm login and npm publish!

And in a few hours it'll appear as a dependabot PR on the rails app repo. Or we can manually trigger it!

welcome[bot] commented 2 years ago

Congrats on merging your first pull request! 🙌🎉⚡️ Your code will likely be published to https://spectralworkbench.org in the next few days. In the meantime, can you tell us your Twitter handle so we can thank you properly? Now that you've completed this, you can help someone else take their first step! See: Public Lab's coding community!

Georjane commented 2 years ago

npm publish

This looks great. Let's merge and then I'll add you, then you should be able to check out your local main branch, run git pull origin main and get the latest main branch, including this PR.

Then you should be able to run npm login and npm publish!

And in a few hours it'll appear as a dependabot PR on the rails app repo. Or we can manually trigger it!

Thanks @jywarren will run the npm publish command once I have the permissions. I have checked into the main branch and pulled.

jywarren commented 2 years ago

OK, I added you! Once you publish, you should momentarily see the version at https://www.npmjs.com/package/spectral-workbench/ go up by 0.0.1!

jywarren commented 2 years ago

Hooray! image

jywarren commented 2 years ago

So triggering dependabot is a bit obsure. But you can do it from this page: https://github.com/publiclab/spectral-workbench/network/updates

jywarren commented 2 years ago

Oh, odd - SWB doesn't have a javascript dependabot setting. I wonder why not? Usually it'll note yarn.lock or package.json, here I see only the Gemfile for Ruby dependencies...

jywarren commented 2 years ago

aha- sorry, actually it was misconfigured - we haven't been getting any JS updates at all! 😱

Not a huge deal, but you can see here, all are Ruby: https://github.com/publiclab/spectral-workbench/issues?q=label%3Adependencies+is%3Aclosed

The difference is we only have bundler (for ruby gems) in the config file: https://github.com/publiclab/spectral-workbench/blob/4592de0b9471e49fc8c1146d9d7e296df5add9ef/.github/dependabot.yml#L3

vs. also including NPM as we do in plots2: https://github.com/publiclab/plots2/blob/5a4937bbc68d6348e0a7f9df05ae0d182c4539a7/.github/dependabot.yml#L46

I'm not sure how this happened but I'll change it now.

jywarren commented 2 years ago

Great - ok done and it's checking now. It might find a bunch of other JS ones that it prioritizes first, i'm not sure how it chooses which.

jywarren commented 2 years ago

Yes and they're starting to appear here: https://github.com/publiclab/spectral-workbench/pulls

What we'll want to do is try installing it locally in a copy of the Rails app, to see if we can pull it in and if anything breaks. Ideally we can test out the thing you tweaked, but not a huge deal if not since it's such a small thing.

Then once we can get the dependabot PR to pass tests (it can happen that we need to make fixes to the JS library and re-release a bugfix to our release, as 0.2.4, but probably not in this case), we merge it, then it'll get auto-published to https://stable.spectralworkbench.org/ and we can test it out there!

There are a lot of steps here, but that's to help ensure things work properly along the way.

jywarren commented 2 years ago

I'm merging some so we get through all the backlog of JS dependabot updates. Many are for simple things or very small updates so we can be pretty sure.

jywarren commented 2 years ago

Well, I'm not sure why dependabot isn't picking it up. It's very low on the list alphabetically and in the listing at https://github.com/publiclab/spectral-workbench/blob/525dfe41e5d0259054c7ac7116caf6d3d4bbfa2c/package.json#L24

...so maybe that's it. We can just do it manually this time, and maybe next time dependabot will have 'caught up'. To do it manually you can check out the main branch of spectral-workbench (the rails app) and then make a feature branch from it, then, update the package.json file to:

"spectral-workbench": "^0.2.3"

That way it'll force it to use at least the newest version. Then run yarn install - and commit the yarn.lock, package.json files in a new PR. Then we can try it out a bit and then merge from there!

jywarren commented 2 years ago

There may be a rate limiting issue with dependabot so it's stopped fetching new version... i'm not totally sure. Sorry about this, it's a little more work than normal since this has not been done in so long!!

jywarren commented 2 years ago

Retriggered Dependabot now, fingers 🤞

jywarren commented 2 years ago

I decided to open this manually in https://github.com/publiclab/spectral-workbench/pull/865 -- will move there!