Replaced Broken Terminal Command With Working Terminal Commands in the CONTRIBUTING.md file #7776
Conversation
|
Want to review this pull request? Take a look at this documentation for a step by step guide! Note that CONTRIBUTING.md cannot previewed locally; rather it should be previewed at this URL: |
github-actions
bot
added
role: front end
|
Review ETA: 12 PM 11/28/2024 |
There was a problem hiding this comment.
Thank you for working on this issue, @dvernon5 !
You're doing these right:
- The branch you made the change on is correct
- The linked issue (original issue) number is included
- The content required to remove and to add is accurate
- The CodeQL Alerts are checked (check box above)
- The URL to the test page is working
Great job!
There was a problem hiding this comment.
Hey @dvernon5 Everything looks good- just a couple comments
- I understand how the instructions for Section 2.7e of CONTRIBUTING.md has code that will not work in terminal #7378 could be read this way; however, the section "For Reviewers: Do not review..." should be an instruction that occurs after you have answered the question "Why did you make the changes (we will use this info to test)?", and not the answer to the question itself like the instructions seem to say. Please add an answer after "Why did you make the changes..." - something like "The CONTRIBUTING.md section 2.7.e has code formatting errors that need to be fixed". and move the "For Reviewers..." down. Something like this might work:
### Why did you make the changes (we will use this info to test)? - The CONTRIBUTING.md section 2.7.e has code formatting errors that need to be fixed ### For Reviewers - Do not review changes locally, rather, review changes at https://github.com/dvernon5/website/blob/replacing-broken-code-command-7378/CONTRIBUTING.md#27e-working-on-an-issue-5-incorporating-changes-from-upstream - Please remove the comment on line 707- we are continuously replacing things in this file and if we added this comment each time, we would have dozens of these
Thanks!
FYI- It is a good idea not to assign people to review your issue unless you know the person is available, or if you need a followup to a review that person previously did. I am saying this because one of the people you assigned is no longer active in the project. Else when people look at your issue they will mistakenly believe you have enough reviewers
|
Hello @t-will-gillis Thank you for your feedback. Re-reading the exercise you are correct, I was supposed to insert the question "For Reviewers" underneath the question "Why did you make the changes" Thank you for the clarification. Also, I made the changes to the initial mistakes on the markdown template, as well as removing the comment on line 707 and re-pushing my local code into the repository. Question: After making a pull request, what should I do to get my issue checked followed by merging if all steps are correct? Should I leave a message in the Slack hfla-site channel to let someone know that my issue is ready to be checked? |
Hi @dvernon5 , developers usually review your PR within 2 to 4 days after your PR is submitted. Sending your PR URL to the hfla-site-pr channel will be a nice reminder for available developers. To merge your PR to the gh-pages branch, you will need at least 2 reviewers to approve your PR. You will need to keep an eye on your PR, if someone leaves you a comment asking for a change, please respond and make the change in time. Once you complete the change requests, you can re-request the reviewers to review your PR by clicking the The reviewers will provide their review ETA and availability after they assign themselves. Please don't hesitate to reach out through the Slack channel or leave a comment in your PR if you have any questions or need help. Hack for LA will have the annual break for a whole month in December and will be back in January. You can leave a message in the Slack channel and some of us will check the message and try to reply occasionally. |
There was a problem hiding this comment.
Hey @dvernon5 Great- thank you for making the changes!
To add to @siyunfeng 's comments (thanks!) - after you make changes requested by a reviewer, be sure to re-request reviews by clicking on the chasing arrows next to each of the reviewers' names:
This will alert the reviewers that they should look at the PR again.
@mchait18 if would you please re-review this?
|
Thank you for your response and clarification. I truly appreciate it and understand the PR process. Thank you! |
|
No problem thank you once again for the suggestions. As well thank you for the further explanation and the screenshot. It truly helps. Thank you for all of your help. |
|
Hi again @mchait18 - No need to re-review since you previously Approved this PR |
Fixes #7378
What changes did you make?
website/CONTRIBUTING.mdsection "2.7e Working on an issue (5): Incorporating changes form upstream"Why did you make the changes (we will use this info to test)?
For Reviewers:
CodeQL Alerts
After the PR has been submitted and the resulting GitHub actions/checks have been completed, developers should check the PR for CodeQL alert annotations.
Check the PR's comments. If present on your PR, the CodeQL alert looks similar as shown
Please let us know that you have checked for CodeQL alerts. Please do not dismiss alerts.
Instructions for resolving CodeQL alerts
If CodeQL alert/annotations appear, refer to How to Resolve CodeQL alerts.
In general, CodeQL alerts should be resolved prior to PR reviews and merging
No visual changes to the website