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

[Planning] - spectralworkbench.js / rails integration #645

Closed
11 tasks done
waridrox opened this issue Jun 5, 2021 · 16 comments
Closed
11 tasks done

[Planning] - spectralworkbench.js / rails integration #645

waridrox opened this issue Jun 5, 2021 · 16 comments
Labels

Comments

@waridrox
Copy link
Member

waridrox commented Jun 5, 2021

Working upon publiclab/spectral-workbench.js#219 to achieve the following:

Open for suggestions!
CC: @jywarren @RuthNjeri @pydevsg

@waridrox waridrox changed the title [Planning [Planning] - spectralworkbench.js / rails integration Jun 5, 2021
@jywarren
Copy link
Member

jywarren commented Jun 7, 2021 via email

@jywarren
Copy link
Member

jywarren commented Jun 8, 2021 via email

@waridrox
Copy link
Member Author

waridrox commented Jun 8, 2021

So I managed to pull the recent changes by pointing the library to the main branch by the cmd bower install 'git://github.com/publiclab/spectral-workbench.js#main' --save and it worked! Also it seems there is no bower.json file present in the codebase. Will try to add it soon...

@jywarren
Copy link
Member

ah, are we still using bower? I thought Spectral Workbench uses Yarn now, and the command would be yarn install. Could you try that as I think we are trying to deprecate bower usage!

Also - it may be best to fix it to a specific commit, as main will change quite often, and so merging to main could cause a test failure on the main branch of the parent app. Pinning it to a specific commit introduces an extra step within this repository to pull in the latest version (Dependabot will help automate this too!) and so we get a look in an independent PR at whether the latest version breaks the tests. Make sense? Thanks @waridrox !!

@jywarren
Copy link
Member

Npm publish docs are here: https://docs.npmjs.com/cli/v6/commands/npm-publish/

Basically the first time or two i'd like to lead the publication process, then you can start doing it.

How familiar are you with semantic versioning? You can read about it here: https://docs.npmjs.com/about-semantic-versioning - remember the discussions in publiclab/image-sequencer#1751 about how this lets us communicate and coordinate around releases?

Thanks! How are things going overall?

@jywarren
Copy link
Member

OK, so i've opened a PR bumping SWB.js to v0.2.0, as the last time it was published was 5 years ago! 😱

https://www.npmjs.com/package/spectral-workbench shows this. Once we merge it, we'll be able to publish a new version. A lot has been added but I don't believe it includes any breaking changes, so I consider it an 0.2.0 version number by semantic versioning. How does that sound?

@jywarren
Copy link
Member

Awesome, i merged it and ran npm publish from the main branch. We are at v0.2.0 now!!!

@jywarren
Copy link
Member

@waridrox we are still a little stuck on getting tests passing on #636 in github actions. Have you explored copying some of the setup from plots2 to get this running? This would unblock a TON of our dependency upgrades, and give your work this summer a lot more support!

@waridrox
Copy link
Member Author

waridrox commented Jun 11, 2021

ah, are we still using bower? I thought Spectral Workbench uses Yarn now, and the command would be yarn install. Could you try that as I think we are trying to deprecate bower usage!

Hi @jywarren, thanks a lot for publishing the npm package 😄, things should be pretty simple now...unfortunately yarn install / npm install lead to an error. So I had to create a bower.json file to get the dependencies installed. But if we are moving away from bower then we should take a look at the licensing part of the dependencies as suggested in the error since as of now only the spectral-workbench.js library dependencies live inside the /node_modules/ folder.

Screenshot 2021-06-11 at 1 32 58 PM

Screenshot 2021-06-11 at 1 31 51 PM

How are things going overall?

Great!! Currently I'm trying to integrate the new capture interface at a capture/v2 URL...and this is the status so far 😅:
Screenshot 2021-06-11 at 2 32 11 PM

we are still a little stuck on getting tests passing on #636 in github actions

Sure I can take a look at this soon after working on the above...Thanks ✨

@jywarren
Copy link
Member

Aha - ok, yes, so let's just ensure we use the right licensing string, we can reference:

https://github.com/publiclab/spectral-workbench.js/blob/4f29a95119af0aef7a4a2e131ec4815df475e1a8/package.json#L20

See how it's different here:

"license": "GPL3",

Let's just change it to match!

@waridrox
Copy link
Member Author

waridrox commented Jun 13, 2021

Hey everyone, I wanted to know if it is a good idea to place the required JS files from the library into the assets folder so that it gets utilised by the asset pipeline / should we point the JS files to the node_modules folder / should we look towards something like webpacker gem. What would be the best possible choice ?

Also regarding the package.json, could we likely create a new one since many of the dependencies have changed their names on NPM...
Here's a list of dependencies ported from the package.json file to direct to their latest compatible version for the app. I've checked the app with these dependencies and it doesn't appear to be breaking in functionality:

"dependencies": {
    "ace-builds": "^1.4.12",
    "bootstrap": "^3.4.1",
    "d3": "^3.5.4",
    "flot": "^0.8.0-alpha",
    "font-awesome": "^4.7.0",
    "getusermedia-js": "^1.0.0",
    "jasmine": "^3.7.0",
    "jasmine-ajax": "^4.0.0",
    "jquery": "^3.3.1",
    "jquery.steps": "^1.0.3",
    "junction": "^0.2.0",
    "leaflet": "^1.7.1",
    "moment": "^2.29.1",
    "nvd3": "^1.7.1",
    "spectral-workbench": "^0.2.0"
  }

Please voice your opinions regarding the same. Thanks
CC: @jywarren @pydevsg @RuthNjeri

@pydevsg
Copy link
Member

pydevsg commented Jun 14, 2021

Hi, @waridrox thanks for pointing them out.
Here are my views on your concerned questions.

  1. It is always preferred to put the js into the assets folder. Also, put it inside javascripts folder.
  2. Coming to package.json , you don't need to create a new package.json file. In order to put the latest version what you can do is "package-name":"*" this will directly install the recent versions.
    In case you want to add a specific version just write it and the latter will be installed.
    Let me know if you need any more help while integrating 😅

@jywarren
Copy link
Member

Hi @waridrox - great questions.

It is always preferred to put the js into the assets folder. Also, put it inside javascripts folder.

This is true, but also Rails has evolved, messily, in terms of the integration of npm, compilers (sprockets earlier, webpacker later) and the Asset Pipeline. You can see that we customized the asset pipeline here just as in plots2, and we'll have to do it again for Rails 6 when that arrives. My feeling is that we should stick with what @alaxalves did in the big #499 #530 upgrades, and just make good plans for a bigger change in Rails 6, which is in the future.

Did you find that the package.json packages are no longer available? Are there some bower-era packages that still aren't represented in package.json, or are you saying some package.json packages have changed names -- and, which?

Thanks!

@jywarren
Copy link
Member

Also, i'd like to ask if you could open a PR with the latest version so we can see how it does in Github Actions and GitPod... i think it'll help @pydevsg myself and you all stay synced! Thank you!!!

@jywarren
Copy link
Member

jywarren commented Jul 3, 2021

Hi @waridrox i was going to say perhaps responsiveness could wait until the functional changes, but it looks like you've already started! 🎉

6: Creation of login prompt for user authentication to the app.

This one could be as simple as a link to /login. However, looking at the existing v1 /capture page, see how it offers to let you use it without a login, but encourages you to log in if you want to save? We should try a modal popup with the same kind of info.

image

@jywarren
Copy link
Member

I'm going to close this up and open a new one for any remaining!

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

No branches or pull requests

3 participants