-
-
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
Deleted the file assets/images/join-us/donation-graphic.svg #5398
Deleted the file assets/images/join-us/donation-graphic.svg #5398
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 9-5 |
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.
Looks good! Join Us page on local page looks the same as on the live webpage. Thanks!
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 @ndmoc — the branching is set up correctly, the corresponding issue was linked, and the requested change of removing the donation-graphic.svg
file was made without affecting the Join Us page, but I have two small requests for you to edit and update your PR.
- As you had noted within the
<details>
tag, the image was unused and did not result in visual changes; since there are no visual changes, can you delete the entire Screenshots section in your PR? This will help keep our codebase documentation clean and it's a bit more obvious at first glance that there are no visual changes from this PR when this is deleted. - The Why of your PR could explain the change a bit more thoroughly; looking at the linked issue, the reason the file was to be deleted is because it is unused and deleting it would "keep the file system clean and avoid confusion". Can you update the Why section of your PR with a bit more explanation, closer to the Overview in the linked issue?
Other than those changes, everything is looking good so far!
Thank you for the feedback Adrian. I removed the screenshot section and added an explanation regarding the issue's overview. I will be more specific moving forward! Thank you |
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.
Thank you for making those changes! The PR looks good to me now; thank you for taking up this issue and contributing to HfLA! 🙌🏼
Fixes #4466
What changes did you make?
Why did you make the changes (we will use this info to test)?