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

Convert to ES6 #49

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Convert to ES6 #49

wants to merge 1 commit into from

Conversation

jantimon
Copy link

hey :)

this PR is a major upgrade for qrcode-terminal:

  • upgrade es5 to es6
  • dosen't change any logic
  • doesn't add any depencies
  • fixes tests (duto sinon upgrade)

ES6 and ESModules require Node12.
I hope that is fine - now that Node14 is the current long term support version: https://nodejs.org/en/about/releases/

@mwbrooks
Copy link
Collaborator

Incredible work @jantimon! Thank you for taking the time to upgrade the package while retaining the logic and avoiding new dependencies.

I'm happy to merge this PR. I'll just give it a few moments to see if @gtanner has anything to add.

@gtanner
Copy link
Owner

gtanner commented Feb 10, 2022

Looks ok to me as well @mwbrooks !

@mwbrooks
Copy link
Collaborator

Awesome @gtanner!

@jantimon I'm running into 2 failed tests on my end and I'm curious if you can help me through it.

$ node --version
v12.19.0

$ npm test
...
  6 passing (92ms)
  2 failing

  1) in the main module
       the generate method
         when not providing a callback
           "after each" hook for "logs to the console":
     TypeError: Cannot read property 'restore' of undefined
      at Context.<anonymous> (test/main.js:13:31)
      at processImmediate (internal/timers.js:461:21)

  2) in the main module
       the generate method
         when providing a callback
           will not call the console.log method:
     Error: expected true to equal false
      at Assertion.assert (node_modules/expect.js/index.js:96:13)
      at Assertion.be.Assertion.equal (node_modules/expect.js/index.js:216:10)
      at Assertion.<computed> [as be] (node_modules/expect.js/index.js:69:24)
      at Context.<anonymous> (test/main.js:32:47)
      at processImmediate (internal/timers.js:461:21)

The first error is that sinon.sandbox.restore is undefined. It looks like a fresh npm install used [email protected]:

cat node_modules/sinon/package.json | grep version
    "postversion": "./scripts/postversion.sh",
    "preversion": "./scripts/preversion.sh",
    "version": "./scripts/version.sh"
  "version": "13.0.1"

@Cansiny0320
Copy link

Hope this pr gets merged

@maukoese
Copy link

Any idea on when this will be merged?

@ctsstc
Copy link

ctsstc commented Apr 5, 2024

Would love to see ES6 support and the updates.

@coolRoger
Copy link

can someone merge this asap?

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

Successfully merging this pull request may close these issues.

7 participants