refactor: automate build with rollup #83

raulfdm · 2019-12-22T14:39:59Z

fixes #82.

Basically the proposal is just add a very simple command which does the build process. Consequently is no need to keep bin/docsify versioned anymore.

Also it may allow you guys to write ES6 code in the future if you want. The only configuration it'll need is https://github.com/rollup/rollup-plugin-babel.

anikethsaha

Good work 🎉
Left few suggestions

.gitignore

main.js

package.json

rollup.config.js

anikethsaha · 2019-12-22T14:54:20Z

Please check why CI is failing.

anikethsaha · 2019-12-22T14:55:49Z

@jamesgeorge007 there is an error with e2e . Please check

jamesgeorge007

Update your branch. Ig that is gonna make the CI green.

anikethsaha · 2019-12-22T15:05:46Z

Also it may allow you guys to write ES6 code in the future if you want. The only configuration it'll need is https://github.com/rollup/rollup-plugin-babel.

We can do this in future PR, adding this in thie PR will create mess

raulfdm · 2019-12-22T15:20:26Z

Also it may allow you guys to write ES6 code in the future if you want. The only configuration it'll need is https://github.com/rollup/rollup-plugin-babel.

We can do this in future PR, adding this in thie PR will create mess

No problem. Just mentioned now it's totally possible and it'll be quite simple to do 😁

anikethsaha

Please check the CI. its still failing. else its good to go

raulfdm · 2019-12-22T15:25:47Z

Pipeline was failing because the tests depends on bin/docsify be there. Since we don't have it versioned anymore, I just add a "postinstall" script to right after installing dependencies, it generates the build.

package.json

anikethsaha · 2019-12-22T15:42:36Z

Why rollup is not bundling the file? it should resolve the dependencies so that we can just deploy the bin folder. 🤔

raulfdm · 2019-12-22T15:44:05Z

@jamesgeorge007 I need a try/catch around the assertions and it's having issues to read the file generated:

Error: Command failed with EACCES: /Users/raul/projects/open-source/docsify-cli/bin/docsify -h
spawn /Users/raul/projects/open-source/docsify-cli/bin/docsify EACCES
    at Process.ChildProcess._handle.onexit (internal/child_process.js:264:19)
    at onErrorNT (internal/child_process.js:456:16)
    at processTicksAndRejections (internal/process/task_queues.js:80:21) {
  errno: 'EACCES',
  code: 'EACCES',
  syscall: 'spawn /Users/raul/projects/open-source/docsify-cli/bin/docsify',
  path: '/Users/raul/projects/open-source/docsify-cli/bin/docsify',
  spawnargs: [ '-h' ],
  originalMessage: 'spawn /Users/raul/projects/open-source/docsify-cli/bin/docsify EACCES',
  shortMessage: 'Command failed with EACCES: /Users/raul/projects/open-source/docsify-cli/bin/docsify -h\n' +
    'spawn /Users/raul/projects/open-source/docsify-cli/bin/docsify EACCES',
  command: '/Users/raul/projects/open-source/docsify-cli/bin/docsify -h',
  exitCode: undefined,
  signal: undefined,
  signalDescription: undefined,
  stdout: '',
  stderr: '',
  failed: true,
  timedOut: false,
  isCanceled: false,
  killed: false
}

anikethsaha · 2019-12-22T15:44:07Z

ava 's test reporter is weird, idk but I dont get the tracked path correctly

anikethsaha · 2019-12-22T15:44:44Z

@jamesgeorge007 I need a try/catch around the assertions and it's having issues to read the file generated:

Error: Command failed with EACCES: /Users/raul/projects/open-source/docsify-cli/bin/docsify -h
spawn /Users/raul/projects/open-source/docsify-cli/bin/docsify EACCES
    at Process.ChildProcess._handle.onexit (internal/child_process.js:264:19)
    at onErrorNT (internal/child_process.js:456:16)
    at processTicksAndRejections (internal/process/task_queues.js:80:21) {
  errno: 'EACCES',
  code: 'EACCES',
  syscall: 'spawn /Users/raul/projects/open-source/docsify-cli/bin/docsify',
  path: '/Users/raul/projects/open-source/docsify-cli/bin/docsify',
  spawnargs: [ '-h' ],
  originalMessage: 'spawn /Users/raul/projects/open-source/docsify-cli/bin/docsify EACCES',
  shortMessage: 'Command failed with EACCES: /Users/raul/projects/open-source/docsify-cli/bin/docsify -h\n' +
    'spawn /Users/raul/projects/open-source/docsify-cli/bin/docsify EACCES',
  command: '/Users/raul/projects/open-source/docsify-cli/bin/docsify -h',
  exitCode: undefined,
  signal: undefined,
  signalDescription: undefined,
  stdout: '',
  stderr: '',
  failed: true,
  timedOut: false,
  isCanceled: false,
  killed: false
}

Yup, errors / exceptions are not handled properly

anikethsaha · 2019-12-22T15:45:44Z

@raulfdm can you try to run the tests with jest. and check

anikethsaha · 2019-12-22T15:46:36Z

I am not a fan of ava anyway, its slower than jest and mocha. Wondering we can migrate soon I guess

raulfdm · 2019-12-22T15:53:57Z

I found the issue. Basically rollup does not generate a proper "executable".

To enable via terminal we should do some chmod. But I found a plugin to enable that by default.

Now probably it'll work.

anikethsaha · 2019-12-22T15:57:02Z

but still the bin/docsify is not bundled code I guess. cause currently, we need those lib files to ship in order to work with it.

anikethsaha · 2019-12-22T15:57:55Z

Cool ... 🎉

raulfdm · 2019-12-22T15:59:08Z

@jamesgeorge007 I need a try/catch around the assertions and it's having issues to read the file generated:

Error: Command failed with EACCES: /Users/raul/projects/open-source/docsify-cli/bin/docsify -h
spawn /Users/raul/projects/open-source/docsify-cli/bin/docsify EACCES
    at Process.ChildProcess._handle.onexit (internal/child_process.js:264:19)
    at onErrorNT (internal/child_process.js:456:16)
    at processTicksAndRejections (internal/process/task_queues.js:80:21) {
  errno: 'EACCES',
  code: 'EACCES',
  syscall: 'spawn /Users/raul/projects/open-source/docsify-cli/bin/docsify',
  path: '/Users/raul/projects/open-source/docsify-cli/bin/docsify',
  spawnargs: [ '-h' ],
  originalMessage: 'spawn /Users/raul/projects/open-source/docsify-cli/bin/docsify EACCES',
  shortMessage: 'Command failed with EACCES: /Users/raul/projects/open-source/docsify-cli/bin/docsify -h\n' +
    'spawn /Users/raul/projects/open-source/docsify-cli/bin/docsify EACCES',
  command: '/Users/raul/projects/open-source/docsify-cli/bin/docsify -h',
  exitCode: undefined,
  signal: undefined,
  signalDescription: undefined,
  stdout: '',
  stderr: '',
  failed: true,
  timedOut: false,
  isCanceled: false,
  killed: false
}

Yup, errors / exceptions are not handled properly

Actually the error is not been handled proper.

I mean, the command is:

// ...rest
test('shows up help message without any args', async t => {
    const { stderr } = await execa(rootCommand, {reject: false});
    t.snapshot(stderr);
});
// ...

Usually async/await is surrounded by a try/catch, but in this case it just throws and ava tries somehow handle it.

I've never use this framework btw, I'm so far satisfied with Jest.

anikethsaha · 2019-12-22T16:00:11Z

I'm so far satisfied with Jest.

true

I hope james will change those in future.

anikethsaha

Great work 🎉

anikethsaha · 2019-12-22T16:26:06Z

Feel free to convert the codebase to es6 but first lets put linters first

jamesgeorge007 · 2019-12-22T16:56:41Z

Ava is lightweight and has got a much minimal API as compared to Jest.
https://github.com/avajs/ava#why-not-mocha-tape-tap

anikethsaha requested changes Dec 22, 2019

View reviewed changes

.gitignore Show resolved Hide resolved

main.js Outdated Show resolved Hide resolved

package.json Show resolved Hide resolved

rollup.config.js Outdated Show resolved Hide resolved

anikethsaha requested a review from jamesgeorge007 December 22, 2019 14:54

jamesgeorge007 requested changes Dec 22, 2019

View reviewed changes

raulfdm force-pushed the master branch from 1569185 to c044abc Compare December 22, 2019 15:13

raulfdm requested review from anikethsaha and jamesgeorge007 December 22, 2019 15:14

raulfdm force-pushed the master branch from c044abc to f2b9dbf Compare December 22, 2019 15:18

raulfdm changed the title ~~WIP: refactor: automate build with rollup~~ refactor: automate build with rollup Dec 22, 2019

anikethsaha requested changes Dec 22, 2019

View reviewed changes

package.json Outdated Show resolved Hide resolved

raulfdm force-pushed the master branch from 949b970 to 988bcab Compare December 22, 2019 15:31

anikethsaha requested changes Dec 22, 2019

View reviewed changes

package.json Outdated Show resolved Hide resolved

raulfdm force-pushed the master branch from 988bcab to b61be6c Compare December 22, 2019 15:39

refactor: automate build with rollup

b072543

raulfdm force-pushed the master branch from b61be6c to b072543 Compare December 22, 2019 15:54

raulfdm requested a review from anikethsaha December 22, 2019 15:59

anikethsaha approved these changes Dec 22, 2019

View reviewed changes

anikethsaha merged commit b8183cd into docsifyjs:master Dec 22, 2019

jamesgeorge007 removed their request for review December 22, 2019 16:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: automate build with rollup #83

refactor: automate build with rollup #83

raulfdm commented Dec 22, 2019 •

edited by anikethsaha

Loading

anikethsaha left a comment

anikethsaha commented Dec 22, 2019

anikethsaha commented Dec 22, 2019

jamesgeorge007 left a comment

anikethsaha commented Dec 22, 2019

raulfdm commented Dec 22, 2019

anikethsaha left a comment

raulfdm commented Dec 22, 2019

anikethsaha commented Dec 22, 2019

raulfdm commented Dec 22, 2019

anikethsaha commented Dec 22, 2019

anikethsaha commented Dec 22, 2019

anikethsaha commented Dec 22, 2019

anikethsaha commented Dec 22, 2019

raulfdm commented Dec 22, 2019

anikethsaha commented Dec 22, 2019

anikethsaha commented Dec 22, 2019

raulfdm commented Dec 22, 2019

anikethsaha commented Dec 22, 2019

anikethsaha left a comment

anikethsaha commented Dec 22, 2019

jamesgeorge007 commented Dec 22, 2019 •

edited

Loading

refactor: automate build with rollup #83

refactor: automate build with rollup #83

Conversation

raulfdm commented Dec 22, 2019 • edited by anikethsaha Loading

anikethsaha left a comment

Choose a reason for hiding this comment

anikethsaha commented Dec 22, 2019

anikethsaha commented Dec 22, 2019

jamesgeorge007 left a comment

Choose a reason for hiding this comment

anikethsaha commented Dec 22, 2019

raulfdm commented Dec 22, 2019

anikethsaha left a comment

Choose a reason for hiding this comment

raulfdm commented Dec 22, 2019

anikethsaha commented Dec 22, 2019

raulfdm commented Dec 22, 2019

anikethsaha commented Dec 22, 2019

anikethsaha commented Dec 22, 2019

anikethsaha commented Dec 22, 2019

anikethsaha commented Dec 22, 2019

raulfdm commented Dec 22, 2019

anikethsaha commented Dec 22, 2019

anikethsaha commented Dec 22, 2019

raulfdm commented Dec 22, 2019

anikethsaha commented Dec 22, 2019

anikethsaha left a comment

Choose a reason for hiding this comment

anikethsaha commented Dec 22, 2019

jamesgeorge007 commented Dec 22, 2019 • edited Loading

raulfdm commented Dec 22, 2019 •

edited by anikethsaha

Loading

jamesgeorge007 commented Dec 22, 2019 •

edited

Loading