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

Change prepack npm script to prepare #1733

Closed
wants to merge 3 commits into from

Conversation

frangio
Copy link
Contributor

@frangio frangio commented Apr 29, 2019

npm allows installing a dependency directly from a git repository, but in those cases the build script needs to run locally on the computer that is installing. The prepack script is supposed to accomplish that but there is bug in npm and it runs without dev dependencies installed (npm/npm#19564). Switching to prepare fixes that. As a side effect, now running npm install in the project will not only install dependencies but also run the build script, but this is not a big deal IMO.

prepare: Run both BEFORE the package is packed and published, on local npm install without any arguments, and when installing git dependencies (See below). This is run AFTER prepublish, but BEFORE prepublishOnly.

prepack: run BEFORE a tarball is packed (on npm pack, npm publish, and when installing git dependencies)

@frangio frangio requested a review from nventuro April 29, 2019 23:35
@nventuro
Copy link
Contributor

Any idea why the CI build is failing? The log looks cryptic.

@frangio
Copy link
Contributor Author

frangio commented Apr 30, 2019

I don't know. 😢 It might be a problem with the cache of dependencies because we changed dependencies in #1732. Let's check after that is merged.

@frangio
Copy link
Contributor Author

frangio commented May 2, 2019

Closing as we were not able to solve the above issue...

@frangio frangio closed this May 2, 2019
@nventuro
Copy link
Contributor

nventuro commented May 3, 2019

Maybe this issue is related? trufflesuite/truffle#1659

@frangio
Copy link
Contributor Author

frangio commented May 3, 2019

Seems to be the same thing, yeah.

@frangio frangio deleted the fix-git-installs branch May 16, 2019 21:20
DrGirlfriend pushed a commit to adobe/aio-cli-plugin-asset-compute that referenced this pull request May 14, 2020
…#7)

- adapt test-worker for aio projects:
  + run tests for all workers by default
  + new option "-a" to select which worker's tests to run
  + add missing "testcase" argument for test-worker
  + error test case if it specifies both a rendition and an expected error
  + improve test log output: timing at the end in separate color (yellow)
  + set request id to testcase name for easier build/test.log reading
- adapt run-worker for aio projects:
  + new option "-a" to select which worker to run
  + not required if there is only one worker
  + exit with non-zero code if it fails
- using "aio app deploy" to build action.zip from manifest instead of
  nui package.json solution based on serverless
- removing serverless and serverless-nui dependencies and related code
- refactoring from promise hell to async/await
- refactoring to split up worker test & run logic in separate pieces:
  + WorkerTestRunner
  + WorkerRunner
  + OpenwhiskActionRunner
  + MockServer, one per mocked file/domain
- refactored into separate functions, clearer error handling
- renamed testfiles.js to cloudfiles.js describing it better
- drop emoji prefixes (e.g. flower) from log messages
- switch oclif manifest creation from "prepack" to "prepare" script to fix installing from git
  npm/npm#19564
  OpenZeppelin/openzeppelin-contracts#1733
- add globby to fix issue with oclif-dev: oclif/oclif#132
- add missing semicolons
- enforce 4 space indentation
- finally fix old issue with repeated logs in build/test.log
- handle ctrl+c and stop any running containers + cleanup directories
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants