Bug fixes, better linux compatibility #9

2022-05-23T20:06:58-06:00

mjforan commented

2022-05-23 20:06:58 -06:00

(Migrated from github.com)

Removed package-lock.json because npm will create it automatically and if we have an existing copy it may complain about mismatched versions
Fixed typo in argument to set browser path
Changed from backslash to forward slash as path separator because it's more friendly to Linux
Added shell=True to subprocess run command to make it work on Linux
Added quotes around URL to download- there was a bug where certain URLs (maybe containing certain special characters?) would halt the command from parsing so then the page name template would overflow to another command
Check if submission.attempt is not None- I'm not sure why it would be None (maybe slight differences in API?) but this would cause an error later: for i in range(submission.attempt)

- Removed package-lock.json because npm will create it automatically and if we have an existing copy it may complain about mismatched versions - Fixed typo in argument to set browser path - Changed from backslash to forward slash as path separator because it's more friendly to Linux - Added `shell=True` to subprocess run command to make it work on Linux - Added quotes around URL to download- there was a bug where certain URLs (maybe containing certain special characters?) would halt the command from parsing so then the page name template would overflow to another command - Check if submission.attempt is not None- I'm not sure why it would be None (maybe slight differences in API?) but this would cause an error later: `for i in range(submission.attempt)`

davekats commented

2022-05-24 14:46:38 -06:00

(Migrated from github.com)

Hi @mjforan, thank you for this contribution! These are some great changes. I wonder if adding a version number to the package.json file will help fix the version mismatch error you were seeing and allow us to keep the package-lock.json file. It's best practice to keep the package-lock.json file checked in to version control. Other than that, this looks great.

Hi @mjforan, thank you for this contribution! These are some great changes. I wonder if adding a version number to the `package.json` file will help fix the version mismatch error you were seeing and allow us to keep the `package-lock.json` file. It's best practice to keep the `package-lock.json` file checked in to version control. Other than that, this looks great.

davekats (Migrated from github.com) approved these changes 2022-05-24 15:09:27 -06:00

mjforan commented

2022-05-24 15:10:57 -06:00

(Migrated from github.com)

I wonder if adding a version number to the package.json file will help fix the version mismatch error you were seeing and allow us to keep the package-lock.json file. It's best practice to keep the package-lock.json file checked in to version control.

Oops I wasn't aware; I added the file back. The issue I saw wasn't major and users can always delete the file on their own, so I wouldn't worry about that.

> I wonder if adding a version number to the `package.json` file will help fix the version mismatch error you were seeing and allow us to keep the `package-lock.json` file. It's best practice to keep the `package-lock.json` file checked in to version control. Oops I wasn't aware; I added the file back. The issue I saw wasn't major and users can always delete the file on their own, so I wouldn't worry about that.

Sign in to join this conversation.

No reviewers

davekats