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

Update README.md to add deploy instructions #1429

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

honeyankit
Copy link
Contributor

@honeyankit honeyankit commented Mar 13, 2025

Context

This pull request includes updates to the README.md file to provide detailed steps for updating a pull request and deploying changes. The most important change is the addition of a new section that outlines the steps to manually update the dependabot-action and ensure that the changes are properly tested and formatted before committing.

Rendered

@honeyankit honeyankit self-assigned this Mar 13, 2025
@honeyankit honeyankit marked this pull request as ready for review March 13, 2025 20:26
@Copilot Copilot bot review requested due to automatic review settings March 13, 2025 20:26
@honeyankit honeyankit requested a review from a team as a code owner March 13, 2025 20:26

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request updates the README.md file to add deploy instructions for updating pull requests and deploying changes.

  • Added a new "Steps to Update Your PR and Deploy Changes" section with terminal command instructions
  • Included a note with an error message block detailing potential CI failure due to uncommitted changes
Comments suppressed due to low confidence (2)

README.md:38

  • The error message text includes embedded markdown hyperlink formatting, which may confuse readers. Remove or simplify these markdown elements to clearly present the CI failure message.
index c09ccea..8f50b37 1006[4](https://github.com/github/dependabot-action/actions/runs/7720200685/job/21044694134?pr=1156#step:7:5)4

README.md:41

  • The error message text contains markdown link markers that detract from its clarity. Remove or reformat these to ensure the error message is presented in straightforward text.
index cc44481..e[5](https://github.com/github/dependabot-action/actions/runs/7720200685/job/21044694134?pr=1156#step:7:6)0840f 100[6](https://github.com/github/dependabot-action/actions/runs/7720200685/job/21044694134?pr=1156#step:7:7)44

Tip: Copilot only keeps its highest confidence comments to reduce noise and keep you focused. Learn more

Copy link
Member

@jeffwidman jeffwidman left a comment

Choose a reason for hiding this comment

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

👍 for making this more explicit.

My only concern is that we're now duplicating in the readme the steps in CI, and so we're introducing a risk they'll go out of sync... the other alternative is to point at the CI file to explain what needs to happen.

That said, these steps are relatively straightforward, so I don't anticipate them changing too frequently.

A few small wording suggestions, none are blockers.

README.md Outdated
Comment on lines 23 to 43
2. Run the following command in your terminal on the same branch used to create the PR whenever you make changes to the PR
```bash
npm run lint-check
npm run format-check -- --write
npm run test
```

```
nvm install;nvm use;npm clean-install;npm ci;npm run package
```
Note: If you do not execute the above step ☝️ and commit the code then CI will fail with the below error:
```bash
Run script/check-diff
Detected uncommitted changes after build:
diff --git a/dist/main/index.js b/dist/main/index.js
index c09ccea..8f50b37 1006[4](https://github.com/github/dependabot-action/actions/runs/7720200685/job/21044694134?pr=1156#step:7:5)4
Binary files a/dist/main/index.js and b/dist/main/index.js differ
diff --git a/dist/main/index.js.map b/dist/main/index.js.map
index cc44481..e[5](https://github.com/github/dependabot-action/actions/runs/7720200685/job/21044694134?pr=1156#step:7:6)0840f 100[6](https://github.com/github/dependabot-action/actions/runs/7720200685/job/21044694134?pr=1156#step:7:7)44
Binary files a/dist/main/index.js.map and b/dist/main/index.js.map differ
```
Copy link
Member

Choose a reason for hiding this comment

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

I suspect you'll want to indent most of this in order to get it to nest properly under 2... have you already played with it in a preview?

```

```
nvm install;nvm use;npm clean-install;npm ci;npm run package
Copy link
Member

Choose a reason for hiding this comment

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

Are all of these needed? I thought in the general case we can assume someone is starting with a clean env and run npm ci... but maybe I'm wrong and the rest of these are needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the codespace, this step is needed.

honeyankit and others added 3 commits March 13, 2025 16:13
Co-authored-by: Jeff Widman <[email protected]>
Co-authored-by: Jeff Widman <[email protected]>
Co-authored-by: Jeff Widman <[email protected]>
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.

3 participants