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

fix(client): check in bundled client code into version control #3524

Merged
merged 1 commit into from
Aug 10, 2020

Conversation

devoto13
Copy link
Collaborator

@devoto13 devoto13 commented May 20, 2020

New script will take care about building assets and also checking that bundled assets are up-to-date as part of CI.

This provides some improvements over the previous approach from f5521df#commitcomment-38967493:

  1. It is possible to install karma from GitHub branch without the assumption that browserify is installed in user's project.
  2. Karma contributors still don't need to run npm run build unless they want to change client code.
  3. Simplifies runtime code.

@karmarunnerbot
Copy link
Member

Build karma 300 failed (commit 5c5ebb3d07 by @devoto13)

@karmarunnerbot
Copy link
Member

Build karma 299 failed (commit 5c5ebb3d07 by @devoto13)

New script will take care about building assets and also checking that bundled assets are up-to-date as part of CI.

This provides some improvements over the previous approach from karma-runner@f5521df#commitcomment-38967493:

1. It is possible to install `karma` from GitHub branch without the assumption that `browserify` is installed in user's project.
1. Karma contributors no longer need to run `npm run build` unless they want to change client code.
1. Simplifies runtime code.
@karmarunnerbot
Copy link
Member

Build karma 301 completed (commit a5407d476f by @devoto13)

@karmarunnerbot
Copy link
Member

Build karma 300 completed (commit a5407d476f by @devoto13)

@devoto13 devoto13 requested a review from johnjbarton May 20, 2020 09:44
@devoto13
Copy link
Collaborator Author

@johnjbarton What do you think about this approach?

@johnjbarton
Copy link
Contributor

Sorry for the delay. I'm just confused by the description. Could you just outline how git-clone-users and contributors work now? It looks like npm build would be needed and it calls browserify so I can't reconcile with the description.

@devoto13
Copy link
Collaborator Author

devoto13 commented Jun 12, 2020

browserify is a development dependency of Karma. So when one installs Karma from the GitHub branch browserify is not installed and therefore it is not possible to use Karma. E.g.

$ npm i https://github.com/karma-runner/karma.git
$ ./node_modules/.bin/karma start
12 06 2020 10:25:08.703:ERROR [karma-server]: Server start failed on port 9876: Error: Cannot find module 'browserify'
Require stack:
- /Users/devoto13/Projects/trash/tmp-karma/node_modules/karma/lib/utils/bundle-utils.js
- /Users/devoto13/Projects/trash/tmp-karma/node_modules/karma/lib/server.js
- /Users/devoto13/Projects/trash/tmp-karma/node_modules/karma/lib/cli.js
- /Users/devoto13/Projects/trash/tmp-karma/node_modules/karma/bin/karma

This is no longer the issue with this commit. So install from GitHub branch use case works out of the box. This allows users to try the potential fix, before we merge it. E.g. #3511 (comment).


As for the contributors, there are no significant changes to the current workflow.

  • If one does not want to modify client code, they don't need to run npm run build and everything works out of the box.
  • If one wants to modify client code, they still need to use npm run build.
  • If one modified the client code, but forgot to run npm run build (or didn't commit generated files), then CI will catch this by running npm run build:check

@johnjbarton johnjbarton merged commit 6cd5a3b into karma-runner:master Aug 10, 2020
karmarunnerbot pushed a commit that referenced this pull request Aug 31, 2020
# [5.2.0](v5.1.1...v5.2.0) (2020-08-31)

### Bug Fixes

* **client:** avoid race between execute and clearContext ([#3452](#3452)) ([8bc5b46](8bc5b46)), closes [#3424](#3424)
* **client:** check in bundled client code into version control ([#3524](#3524)) ([6cd5a3b](6cd5a3b)), closes [/github.com/karma-runner/karma/commit/f5521df7df5cd1201b5dce28dc4e326b1ffc41fd#commitcomment-38967493](https://github.com//github.com/karma-runner/karma/commit/f5521df7df5cd1201b5dce28dc4e326b1ffc41fd/issues/commitcomment-38967493)
* **dependencies:** update dependencies ([#3543](#3543)) ([5db46b7](5db46b7))
* **docs:** Update 03-how-it-works.md ([#3539](#3539)) ([e7cf7b1](e7cf7b1))
* **server:** log error when file loading or preprocessing fails ([#3540](#3540)) ([fc2fd61](fc2fd61))

### Features

* **server:** allow 'exit' listeners to set exit code ([#3541](#3541)) ([7a94d33](7a94d33))
@karmarunnerbot
Copy link
Member

🎉 This PR is included in version 5.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@devoto13 devoto13 deleted the build branch September 1, 2020 12:52
anthony-redFox pushed a commit to anthony-redFox/karma that referenced this pull request May 16, 2023
…-runner#3524)

New script will take care about building assets and also checking that bundled assets are up-to-date as part of CI.

This provides some improvements over the previous approach from karma-runner@f5521df#commitcomment-38967493:

1. It is possible to install `karma` from GitHub branch without the assumption that `browserify` is installed in user's project.
1. Karma contributors no longer need to run `npm run build` unless they want to change client code.
1. Simplifies runtime code.
anthony-redFox pushed a commit to anthony-redFox/karma that referenced this pull request May 16, 2023
# [5.2.0](karma-runner/karma@v5.1.1...v5.2.0) (2020-08-31)

### Bug Fixes

* **client:** avoid race between execute and clearContext ([karma-runner#3452](karma-runner#3452)) ([8bc5b46](karma-runner@8bc5b46)), closes [karma-runner#3424](karma-runner#3424)
* **client:** check in bundled client code into version control ([karma-runner#3524](karma-runner#3524)) ([6cd5a3b](karma-runner@6cd5a3b)), closes [/github.com/karma-runner/karma/commit/f5521df7df5cd1201b5dce28dc4e326b1ffc41fd#commitcomment-38967493](https://github.com//github.com/karma-runner/karma/commit/f5521df7df5cd1201b5dce28dc4e326b1ffc41fd/issues/commitcomment-38967493)
* **dependencies:** update dependencies ([karma-runner#3543](karma-runner#3543)) ([5db46b7](karma-runner@5db46b7))
* **docs:** Update 03-how-it-works.md ([karma-runner#3539](karma-runner#3539)) ([e7cf7b1](karma-runner@e7cf7b1))
* **server:** log error when file loading or preprocessing fails ([karma-runner#3540](karma-runner#3540)) ([fc2fd61](karma-runner@fc2fd61))

### Features

* **server:** allow 'exit' listeners to set exit code ([karma-runner#3541](karma-runner#3541)) ([7a94d33](karma-runner@7a94d33))
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