-
-
Notifications
You must be signed in to change notification settings - Fork 786
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 github-handle for Sara Brady in tech-work-experience.md #7312
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.
|
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 @joooseph2 everything looks great.
Branches are correct, there's a linked issue and you correctly stated that there are no visual changes done to the website. No changes in my docker version also.
When it comes to the code, github-handle:
was correctly added underneath - name: Sara Brady
without using tab indent.
Great job! Thanks for working on this
Review ETA: 6 AM 8/19/24 |
Review ETA: 8/20/24 3PM |
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 @joooseph2 Good Work!
What was completed correctly:
To and from branches
linked issue
what and why are clear and thorough
No visual changes made / correctly indented without tab
Thanks for completing this issue!
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 @joooseph2 looking in more detail to your descriptions.
I'm a bit confused by the answer to Why did you make the changes.
I think that the issue states a different reason as to Why adding the github-handle
variable is done.
Moreover, in the What section we are not changing a variable but rather adding a new one.
Would you like to try answering the questions one more time?
@santisecco We need to create a single variable github-handle to hold the github handle for each member of the leadership team. Eventually github-handle will replace the github and picture variables, reducing redundancy in the project file. Here's my summary as to why to submit the second pull request. The issue was created to replace the github: and picture: variables with github-handle: to reduce redundancy. Personally, I think it's fine. The issue says to replace |
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.
Agree @joooseph2 I meant that, we are "adding the variable to hold the github-handle of the member". It's a simple explanation, perhaps redundant, but we understand "what" the github-handle is, and the fact that one day an automation might be created to add them all up.
That would be my personal answer, but what you are saying is also true.
Thanks for clarifying and explaining that you also saw that it's "eventual"
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 @joooseph2, I reviewed the changes, and everything looks great. The github-handle:
was correctly added under name: Sara Brady
without using tab indentations. There are no visual changes to the website, which aligns with the expectations for this change. Nice work!
Fixes #7245
What changes did you make?
Changed variable
name:
in /website/_projects/tech-work-experience.md to include variablegithub-handle:
name: Sara Brady
name: Sara Brady github-handle:
Why did you make the changes (we will use this info to test)?
Screenshots of Proposed Changes Of The Website (if any, please do not screen shot code changes)