Add "github-handle" variable for Anousha Shadrach in tdm-calculator.md#7234
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. |
|
Availability: M - F 12pm - 5pm |
Anahisv23
left a comment
There was a problem hiding this comment.
Hi @minkang3, thanks for working on this ticket. Here's what you did well along with some feedback. Nice job!
What went well
- Into and from branch are correct
- There is a linked issue present
- Modified the appropriate lines of code to add
github-handlevariable
Feedback
- In your PR it would be helpful to be specific and descriptive about why you did the changes you did or mention what the goal of the ticket was.
|
@Anahisv23 Thank you for the feedback, I'll keep that in mind for my next contribution. |
|
Hi @minkang3, could you please update this PR so its more specific and descriptive about why you did the changes you did or mention what the goal of the ticket was. The merge team will be the ones to close your PR. I think you should wait to update the status to done until the merge team has looked at your PR. Great questions! |
daras-cu
left a comment
There was a problem hiding this comment.
Hi @minkang3, thanks for working on this issue! Your changes look good, there's just a couple small things to adjust.
Here's what looks good:
- Your branches are set up correctly for merging.
- You linked the original issue in the PR.
- The code changes were made as described in the original issue.
- When testing the changes locally, the page displays correctly with no visual changes.
To do:
- As mentioned, the "Why did you make the changes" section should show that you understand the purpose of the changes you made. You can usually find this described in the original issue, in this case the new variable is going to replace two other variables in order to reduce redundancy in the project file, so your answer could be as simple as "To reduce redundancy in the file."
- Since there are no visual changes to the website, you can delete both the details sections with screenshots and just put "No visual changes."
After you make the two changes, let me know in a comment or re-request a review and I'll come back and take another look!
Fixes #7174
What changes did you make?
Why did you make the changes (we will use this info to test)?
githubandpicturevariables were able to be condensed, reducing the redundancy in the project file.Screenshots of Proposed Changes Of The Website (if any, please do not screen shot code changes)
No visual changes.