-
-
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
Add Allison Profile to 311 Data Page #7639
Add Allison Profile to 311 Data Page #7639
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: 10/26/2024 Sat |
Review ETA: 10/28/2024, 11:59pmPST |
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.
Nice job working on this issue, @trimakichan !
You provided your availability and ETA in the original issue, created a new branch to make the changes, and included the linked issue in your PR.
The answers to What changes did you make?
and Why did you make the changes (we will use this info to test)?
are clear so our fellow developers would have a better understanding of the issue and your schedule. The changes you made are accurate. I appreciate that you provided the before-and-after screenshots to show the changes you made.
Improvement:
- Name the branch with more specific details, such as
add-allison-jeon-profile-311-data-7542
Thank you for your contribution!
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.
Hey @trimakichan great job on this issue.
- The branch name is good with the issue number
- The issue is linked correctly
- The changes are displayed correctly
One thing you need to do is on the original issue you need to tick one of the boxes that says : Verify the changes by viewing the following in your local environment and include before and after screenshots with your pull request:
That will complete this issue. Thank you for your contribution. Keep it up!
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.
Hi @trimakichan,
Looks great! Things you did well:
- branching naming to and from
- changes made and why they were made stated clearly
- visual before and after provided
- code changes are correct
Thank you for your contribution!
Thank you so much for taking the time to check this issue, @siyunfeng! Your advice on naming the branch is helpful. Thanks! |
Thank you for checking, @codyyjxn! I checked the box you mentioned. The next step is just to wait for my pull request to be merged, correct? |
Yes, once this PR is merged, please fill out the info in If this is your first |
Thank you for checking! I appreciate you taking the time to do so, @terrencejihoonjung. |
Thank you for your quick reply and for describing the next step very clearly, @siyunfeng! I appreciate it. |
@trimakichan Thank you for working on this issue. |
Fixes #7542
What changes did you make?
Why did you make the changes (we will use this info to test)?
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