-
-
Notifications
You must be signed in to change notification settings - Fork 778
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
Added Member Nooria Ali To Civic Tech Jobs #7773
Added Member Nooria Ali To Civic Tech Jobs #7773
Conversation
Want to review this pull request? Take a look at this documentation for a step by step guide! From your project repository, check out a new branch and test the changes.
|
Review ETA: 12 PM 11/27/24 |
Review ETA: 27 Nov @ 6 PM (GMT) |
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.
PR Review
@dvernon5 Thank you for your PR.
Things that went well
- Providing Availability & ETA in the issue.
- Good PR title, issue number is added, what changes you made and why are clear, good screenshots, and CodeQL alerts have been checked.
- Branch name is ok but can be better.
- I can see the changes in my local environment.
- All links are working fine e.g. GitHub and Slack.
Things to change
I have approved this PR because the code is fine but please make the following changes:
- In the comments on the changes in the Pull Request for your particular file path use backticks
(`)
to enclose the file path such as_projects/expunge-assist.md
. - For the PR title: it is 'Civic Tech Jobs' not 'Civic Tech Job'. Please rename PR title.
Suggestion (for your next task): the branch name could be better by using a more descriptive branch name such as Civic-Tech-Jobs-add-Nooria-Ali-7730
.
Thank you. I appreciate your work. Feel free to ask if you have any questions.
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.
Hello @dvernon5
- The code changes look good.
- The issue is linked correctly
- CodeQL alerts are checked
- The original issue items are checked
- The content added is correct
- Before and After photos are accurate
Great job
Thank you for your feedback. I took your advice and made the following changes. Also, I will be more descriptive of my branch name for the next issue. Once again thank you for all of your suggestions. |
Thank you for your time and feedback. I truly appreciate it. |
Review ETA: 12 PM 11/27/24 |
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.
Amazing job!
- Issue number present
- Images are perfect
- Code added is clear and concise
Thank you for taking care of the issues stated by @mugdhchauhan
Great Job and Happy Thanksgiving!
Review ETA: noon 11/27/24 |
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.
This looks great!
-You completed it in the right branch
-You linked the issue
-You understand the issue and executed well
-Included visuals
-Made the recently requested updates from other reviews!
Thank you!
Fixes #7730
What changes did you make?
_project/civic-tech-jobs.md
Why did you make the changes (we will use this info to test)?
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
Screenshots of Proposed Changes To The Website (if any, please do not include screenshots of code changes)
Visuals before changes are applied
Visuals after changes are applied