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.

Sign up for GitHub

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 Build Command (supersedes #4848) #4861

Closed
wants to merge 17 commits into from
Closed

Fix Build Command (supersedes #4848) #4861

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
e402865
Use && instead of ; to join composite commands
wbt Mar 14, 2022
771c2b2
npm i rimraf --save-dev
wbt Mar 14, 2022
cf7a047
Change command to run rimraf
wbt Mar 14, 2022
b875b14
npm i
wbt Mar 14, 2022
8147b4d
Cut redundant command covered in test:cov
wbt Mar 14, 2022
0f1eb0c
Split checklist item into 2
wbt Mar 14, 2022
92cf679
Use 'have' for grammatical consistency
wbt Mar 14, 2022
ce80a9c
Add bash keyword before script source
wbt Mar 14, 2022
9f790fd
Add line to changelog with PR #
wbt Mar 14, 2022
9779e6f
Merge branch '1.x' into wyatt/1.x/fix-build-command
spacesailor24 Mar 17, 2022
5e04fba
Ran npm i
spacesailor24 Mar 17, 2022
0f44bdd
Merge branch '1.x' into wyatt/1.x/fix-build-command
spacesailor24 Mar 18, 2022
0fd25e3
Update CHANGELOG.md
spacesailor24 Mar 18, 2022
26e9cf5
Remove all package-lock.jsons
spacesailor24 Mar 25, 2022
27be3d7
Regenerate all package-lock.jsons
spacesailor24 Mar 25, 2022
f1f369a
Merge branch 'wyatt/1.x/fix-build-command' of github.com:ChainSafe/we…
spacesailor24 Mar 25, 2022
28bf2cd
Merge branch '1.x' into wyatt/1.x/fix-build-command
spacesailor24 Mar 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/PULL_REQUEST_TEMPLATE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ Fixes #(issue)
- [ ] My changes generate no new warnings.
- [ ] Any dependent changes have been merged and published in downstream modules.
- [ ] I ran `npm run dtslint` with success and extended the tests and types if necessary.
- [ ] I ran `npm run test:unit` with success.
- [ ] I ran `npm run test:cov` and my test cases cover all the lines and branches of the added code.
- [ ] I ran `npm run build` and tested `dist/web3.min.js` in a browser.
- [ ] I ran `npm run build` with success.
- [ ] I have tested the built `dist/web3.min.js` in a browser.
- [ ] I have tested my code on the live network.
- [ ] I have checked the Deploy Preview and it looks correct.
- [ ] I have updated the `CHANGELOG.md` file in the root folder.
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -525,6 +525,12 @@ Released with 1.0.0-beta.37 code base.

## [1.7.2]

### Changed

- Improve npm script commands for development purposes (#4848)



### Changed
- Remove deprecated `close` event listener (#4825) (#4839)

Expand Down
89,497 changes: 48,506 additions & 40,991 deletions package-lock.json
View file
Open in desktop

Large diffs are not rendered by default.

35 changes: 18 additions & 17 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,27 +23,27 @@
"lint": "jshint *.js packages",
"test": "mocha -R spec --require ts-node/register --grep E2E --invert",
"test:unit": "nyc --no-clean --silent _mocha -- -R spec --require ts-node/register --grep E2E --invert --exit",
"test:cov": "npm run cov:clean; npm run test:unit; npm run test:e2e:clients; npm run cov:merge_reports",
"test:cov": "npm run cov:clean && npm run test:unit && npm run test:e2e:clients && npm run cov:merge_reports",
"dtslint": "lerna run dtslint",
"depcheck": "lerna exec dependency-check -- --missing --verbose .",
"bundlesize": "bundlesize || true",
"geth": "geth-dev-assistant --accounts 5 --tag stable --gasLimit 7000000",
"test:e2e:ganache": "./scripts/e2e.ganache.sh",
"test:e2e:geth:auto": "./scripts/e2e.geth.automine.sh",
"test:e2e:geth:insta": "./scripts/e2e.geth.instamine.sh",
"test:e2e:clients": "npm run test:e2e:ganache; npm run test:e2e:geth:insta; npm run test:e2e:geth:auto",
"test:e2e:chrome": "./scripts/e2e.chrome.sh",
"test:e2e:firefox": "./scripts/e2e.firefox.sh",
"test:e2e:min": "./scripts/e2e.min.sh",
"test:e2e:cdn": "./scripts/e2e.cdn.sh",
"test:e2e:browsers": "npm run build; npm run test:e2e:chrome; npm run test:e2e:firefox",
"test:e2e:publish": "./scripts/e2e.npm.publish.sh",
"test:e2e:truffle": "./scripts/e2e.truffle.sh",
"test:e2e:mosaic": "./scripts/e2e.mosaic.sh",
"test:e2e:ganache:core": "./scripts/e2e.ganache.core.sh",
"test:e2e:gnosis:dex": "./scripts/e2e.gnosis.dex.sh",
"ci": "./scripts/ci.sh",
"cov:clean": "rm -rf .nyc_output; rm -rf coverage",
"test:e2e:ganache": "bash ./scripts/e2e.ganache.sh",
"test:e2e:geth:auto": "bash ./scripts/e2e.geth.automine.sh",
"test:e2e:geth:insta": "bash ./scripts/e2e.geth.instamine.sh",
"test:e2e:clients": "npm run test:e2e:ganache && npm run test:e2e:geth:insta && npm run test:e2e:geth:auto",
"test:e2e:chrome": "bash ./scripts/e2e.chrome.sh",
"test:e2e:firefox": "bash ./scripts/e2e.firefox.sh",
"test:e2e:min": "bash ./scripts/e2e.min.sh",
"test:e2e:cdn": "bash ./scripts/e2e.cdn.sh",
"test:e2e:browsers": "npm run build && npm run test:e2e:chrome && npm run test:e2e:firefox",
"test:e2e:publish": "bash ./scripts/e2e.npm.publish.sh",
"test:e2e:truffle": "bash ./scripts/e2e.truffle.sh",
"test:e2e:mosaic": "bash ./scripts/e2e.mosaic.sh",
"test:e2e:ganache:core": "bash ./scripts/e2e.ganache.core.sh",
"test:e2e:gnosis:dex": "bash ./scripts/e2e.gnosis.dex.sh",
"ci": "bash ./scripts/ci.sh",
"cov:clean": "rimraf .nyc_output && rimraf coverage",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"cov:clean": "rimraf .nyc_output && rimraf coverage",
"cov:clean": "rm -rf .nyc_output; rm -rf coverage",

Copy link
Contributor

Choose a reason for hiding this comment

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

The suggested change actively reduces cross-platform compatibility. Can you explain why it's important to avoid any changes to package-lock?

Copy link
Contributor

Choose a reason for hiding this comment

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

@wbt We include changes in package-lock files via dedicated and internally initialized PRs only.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is now formally an internally initialized PR. Going out of your way to reduce cross-platform compatibility and break the command on some platforms just doesn't seem like the right direction.

Copy link
Contributor

Choose a reason for hiding this comment

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

@wbt We include changes in package-lock files via dedicated and internally initialized PRs only.

Internal and also dedicated PRs for package lock changes. Thanks

Copy link
Contributor

@wbt wbt Apr 1, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

However, the version of this PR which does not change all the package-lock files this one does was rejected at #4848. In theory, I believe that one could be reopened and merged, but @spacesailor24 seems to prefer the version with all the additional package-lock changes while @jdevcs strongly disagrees, and meanwhile the command nominally required to be run for every PR remains broken.

Copy link
Contributor

Choose a reason for hiding this comment

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

@wbt , we discussed this internally that @spacesailor24 will re generate package lock files in this PR, but there are errors when he tried. #4861 (comment)

Our team is occupied in 4.x so could you investigate and share it.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jdevcs can you just reopen #4848?

Copy link
Contributor

Choose a reason for hiding this comment

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

Its already merged so couldn't reopen.

Copy link
Contributor

Choose a reason for hiding this comment

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

Replaced with #4939.

"cov:merge_reports": "nyc report --reporter=lcov --reporter=html"
},
"repository": {
Expand Down Expand Up @@ -127,6 +127,7 @@
"mocha": "^6.2.3",
"nyc": "^14.1.1",
"pify": "^4.0.1",
"rimraf": "^3.0.2",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"rimraf": "^3.0.2",

"sandboxed-module": "^2.0.4",
"ts-node": "^9.0.0",
"typescript": "^3.9.7",
Expand Down
14 changes: 7 additions & 7 deletions packages/web3-bzz/package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 7 additions & 7 deletions packages/web3-core-helpers/package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/web3-core-method/package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

30 changes: 0 additions & 30 deletions packages/web3-core-promievent/package-lock.json
View file
Open in desktop

This file was deleted.

Loading