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

Initial commit #2

Merged
merged 4 commits into from
Jun 20, 2022
Merged

Initial commit #2

merged 4 commits into from
Jun 20, 2022

Conversation

neurolabusc
Copy link
Member

No description provided.

```
$ git clone https://github.com/tee-ar-ex/trx-javascript
$ cd trx-javascript
$ npm install gl-matrix fflate fzstd
Copy link
Contributor

@frheault frheault May 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to write what is the correct version of Node?
I used sudo apt and the version was so old the code kept crashing on the first line.
When I installed it directly from a .tar.gz from their website it worked immediately.

Something like?
Install the appropriate version of the most recent [NodeJS](https://nodejs.org/en/download/).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@neurolabusc : do you want to address this question in this PR, or should we merge and add this as an issue for future work?

@neurolabusc
Copy link
Member Author

@frheault one nice thing about Debian releases is they freeze software to make user experience consistent. This can also be frustrating when the user wants new features. I have updated the readme as you suggested. Note that node is only required for using the javascript from the command line. The code is compatible with all modern web browsers, as demonstrated by the NiiVue live demo.

@neurolabusc
Copy link
Member Author

@frheault I do think the revised PR addresses your concerns. If you agree, can you accept and apply this pull request.

Copy link
Contributor

@arokem arokem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of small documentation suggestions/fixes from my end. How was the example file generated? Would it make sense to also put it on figshare? See also discussions surrounding tee-ar-ex/trx-python#32

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@neurolabusc
Copy link
Member Author

@arokem I am happy with all your changes. Feel free to apply them and submit this initial PR.

I do plan to make testing datasets available, but was waiting for a decision on the final format. My reader will work fine with or without the proposed modification, but I want the exemplar data to reflect the finalized format.

@frheault frheault merged commit d385ced into tee-ar-ex:main Jun 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants