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

Allow specifying the PDFJS_NEXT build flag via an environment variable when running the various gulp commands #8463

Merged
merged 1 commit into from
May 31, 2017

Conversation

Snuffleupagus
Copy link
Collaborator

After PR #8459, the run-time of the various gulp test commands has regressed quite badly on Windows. For me, gulp test now takes approximately twice as long when run locally on Windows.
The problem seems to be the Babel transpilation step, which takes well over five minutes to run.[1]

For someone like me, who runs tests a lot locally, this slowdown is really hurting the overall development experience.
To get around this I tested setting PDFJS_NEXT = true in gulpfile.js, since the transpilation step isn't necessary when testing in a modern browser.

However, having to edit gulpfile.js every time that I need to run tests isn't very practical. Hence this patch, which adds an environment variable that allows you to disable the transpilation simply by using e.g. PDFJS_NEXT=true gulp test.

I hope that this can be considered an acceptable solution, such that I don't need to maintain this patch locally (or worse, edit gulpfile.js locally before testing).


[1] This can also be observed on the Windows bot, but it seems fine on Linux.

@Snuffleupagus Snuffleupagus changed the title Allow specifying the PDFJS_NEXT build flag via an environment variable when running the various `gulp´ commands Allow specifying the PDFJS_NEXT build flag via an environment variable when running the various gulp commands May 31, 2017
Copy link
Contributor

@yurydelendik yurydelendik left a comment

Choose a reason for hiding this comment

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

Looks good. Can you split that into two lines to add var pdfjsNext = before return?

…ble when running the various `gulp` commands

After PR 8459, the run-time of the various `gulp` test commands has regressed quite badly on Windows. For me, `gulp test` now takes approximately *twice* as long when run locally on Windows.
The problem seems to be the Babel transpilation step, which takes well over five minutes to run.[1]

For someone like me, who runs tests a lot locally, this slowdown is really hurting the overall development experience.
To get around this I tested setting `PDFJS_NEXT = true` in `gulpfile.js`, since the transpilation step isn't necessary when testing in a modern browser.

However, having to edit `gulpfile.js` every time that I need to run tests isn't very practical. Hence this patch, which adds an environment variable that allows you to disable the transpilation simply by using e.g. `PDFJS_NEXT=true gulp test`.

I hope that this can be considered an acceptable solution, such that I don't need to maintain this patch locally (or worse, edit `gulpfile.js` locally before testing).

---
[1] This can also be observed on the Windows bot, but it seems fine on Linux.
@Snuffleupagus
Copy link
Collaborator Author

Looks good. Can you split that into two lines to add var pdfjsNext = before return?

That's a lot better, fixed now, thanks!

/botio-linux lint

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

Command cmd_lint from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.67.70.0:8877/1fc7c6f10249f9b/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Success

Full output at http://54.67.70.0:8877/1fc7c6f10249f9b/output.txt

Total script time: 0.96 mins

  • Lint: Passed

@Snuffleupagus Snuffleupagus merged commit 09d46e9 into mozilla:master May 31, 2017
@Snuffleupagus Snuffleupagus deleted the PDFJS_NEXT-env branch May 31, 2017 15:43
movsb pushed a commit to movsb/pdf.js that referenced this pull request Jul 14, 2018
Allow specifying the `PDFJS_NEXT` build flag via an environment variable when running the various `gulp` commands
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants