-
Notifications
You must be signed in to change notification settings - Fork 779
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
Discourage force pushes #7960
Discourage force pushes #7960
Conversation
|
A wrangler prerelease is available for testing. You can install this latest build in your project with: npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13031892866/npm-package-wrangler-7960 You can reference the automatically updated head of this PR with: npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/7960/npm-package-wrangler-7960 Or you can use npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13031892866/npm-package-wrangler-7960 dev path/to/script.js Additional artifacts:cloudflare-workers-bindings-extension: wget https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13031892866/npm-package-cloudflare-workers-bindings-extension-7960 -O ./cloudflare-workers-bindings-extension.0.0.0-vccbc3eae0.vsix && code --install-extension ./cloudflare-workers-bindings-extension.0.0.0-vccbc3eae0.vsix create-cloudflare: npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13031892866/npm-package-create-cloudflare-7960 --no-auto-update @cloudflare/kv-asset-handler: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13031892866/npm-package-cloudflare-kv-asset-handler-7960 miniflare: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13031892866/npm-package-miniflare-7960 @cloudflare/pages-shared: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13031892866/npm-package-cloudflare-pages-shared-7960 @cloudflare/unenv-preset: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13031892866/npm-package-cloudflare-unenv-preset-7960 @cloudflare/vite-plugin: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13031892866/npm-package-cloudflare-vite-plugin-7960 @cloudflare/vitest-pool-workers: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13031892866/npm-package-cloudflare-vitest-pool-workers-7960 @cloudflare/workers-editor-shared: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13031892866/npm-package-cloudflare-workers-editor-shared-7960 @cloudflare/workers-shared: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13031892866/npm-package-cloudflare-workers-shared-7960 @cloudflare/workflows-shared: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13031892866/npm-package-cloudflare-workflows-shared-7960 Note that these links will no longer work once the GitHub Actions artifact expires.
Please ensure constraints are pinned, and |
CONTRIBUTING.md
Outdated
@@ -202,6 +202,7 @@ Tests in a workspace are executed, by [Vitest](https://vitest.dev/), which is co | |||
|
|||
Every change you make should be stored in a [git commit](https://github.com/git-guides/git-commit). | |||
Changes should be committed to a new local branch, which then gets pushed to your fork of the repository on GitHub. | |||
In general, we discourage force pushing to branches, so please try and make your changes in additive commits. This makes reviewing easier, and is especially helpful when a PR goes through multiple rounds of review for a reviewer to be able to quickly see what's changed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Force pushes are not strictly the problem. For example you must force push when you rebase your PR on main
.
What we want to discourage in prematurely updating or squashing commits that have already been reviewed.
The best practice is to use --fixup
commits IMO, which helps to show which commits new changes would have affected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, during the period before the PR is marked as "ready for review" and someone has actually done a first review, we should actively encourage people to clean up and squash their commits into sensible changes, with force pushes as necessary, so that it is easier for the first reviewer to follow along.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Much better!
No description provided.