-
-
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
Update Public Tree Map Slack Link #6514 #6617
Update Public Tree Map Slack Link #6514 #6617
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.
Note that CONTRIBUTING.md cannot previewed locally; rather it should be previewed at this URL:
|
@gaylem just added the labels. Thanks for the feedback! |
Review ETA: EOD 04/13/2024 |
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 @elisetvy, thank you for submitting this pull request.
You did great on:
- choosing the correct branch for pull request
- linking the issue
- making the slack url change as needed for the issue
- making no visible changes on website for this link change
However, I did find an issue with your request:
- unnecessary changes at line 9 and line 50 for file
public-tree-map.md
Review guideline requires changes to be applicable and clean, and unfortunately this does not seem to be met.
Please revert the changes at line 9 and line 50 in the file public-tree-map.md
to their original state. Once you've made these adjustments, please push the updated changes to your branch, and I will be happy to review it again.
Thank you for your efforts and cooperation in keeping our project's changes clean and efficient!
@tony1ee hi Tony! Thank you for the feedback. Is using 'git revert [id]' the best way to revert a change? I made two changes/commits for this issue. The second change was because I accidentally mistyped something the first time. Should I revert both of these commits and then make the necessary changes? Also curious what would have caused those unnecessary changes in the first place because I didn't touch those lines, but I did notice them get "changed" in the commit history. |
Hi @elisetvy, you can choose to add another commit that fixes line 9 and line 50 so they appear not changed compared to After you push it to Thank you for your efforts, looking forward to your updates! |
To answer your question, one potential reason for these changes made without your explicit actions might be you enabled "format on save" feature on your code editor. |
@tony1ee hi Tony, I just made a new commit to negate the unnecessary changes. I think they were caused by me having format on save on, as you said - thanks for the explanation + advice! |
I made a few commits while still wrestling with my formatter. I think all unnecessary changes have been reverted now. |
Hi @Beatriz-G! Thanks for volunteering to review this issue! When you have a minute, please add your ETA and Availability. Thanks! |
ETA: EOD 04/17/2024 |
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.
Line 34 matches requested changes. Updated public link tree map from messages to archives.
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.
Thanks @elisetvy for the effort of making the requested changes, everything looks good now and I am happy to approve this pull request!
Fixes #6514
What changes did you make?
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)
Updating Slack link. No visual changes to the website.