-
Notifications
You must be signed in to change notification settings - Fork 42
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
Adding profile photo #122
base: gh-pages
Are you sure you want to change the base?
Adding profile photo #122
Conversation
Hi @simran054, can you explain why a 1.59MB picture was added, then removed, then added again? @AbdulMuhaymin can you check the dimensions and decide whether or not to approve the pull request? I think the space between the bottom of the chin and the bottom of the photo may be a bit too big, and the recommendation was always to follow the examples of my picture and Greg's, without over-doing it like in Diya's picture (a bit gets cut off when the picture goes on the website, it's best to look at the photos of myself and Greg, in the "images" folder, rather than looking at the photos on the website). |
Hi @ndattani sorry about that. Initially, I didn't add the image to the images folder, and when I tried to move it, I wasn't able to, so instead, I removed and re-added to the correct folder. If the request can be rejected, I can make some more changes to the cropping and upload the image once more. |
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.
@simran054 I reviewed your commit. As suggested by @ndattani , please crop the image to remove the space between the bottom of your chin and the bottom of the image, and also remove some of the lateral empty space too.
Secondly, this is quite a high resolution image. The dimension that we were using is 270 x 285. Your image has a dimension of 1686 x 1779. That means you have the aspect ratio correct but not the exact value. I am not completely sure but I guess you should also reduce the dimension to 270 x 285 to match with others. So, removing some of the lateral space will also help you maintaining the aspect ratio after you remove the empty space below your chin.
In any case, do not delete the local repository, close this pull request or start a new pull request, please. After you made the suggested changes (cropping maintaining aspect ratio and centering, and then resizing to the desired size), you can commit and then push those changes. The change will be reflected in this pull request.
Hi @simran054, in the future if you don't know how to do something (such as moving a file from one folder to another folder), you an ask it in our "Git" channel in the HPQC Discord server (https://discord.gg/4B59r7ukhV). The command |
@AbdulMuhaymin Thank you for the feedback; I made those adjustments. Please let me know if any further changes are required. Thank you both for your patience :) |
@simran054 Thank you. The image SimranBhalla_fixed.png has correct dimension and I think it satisfies all the criteria except one, i.e., the naming of the file. It's very easy to fix. You do not need to keep the high resolution image. You can delete that image (or move to someplace else in your computer other than this repository) and rename this newly uploaded fixed version as SimranBhalla.png. In this way, there will be only 1 file change from your side in this PR. And since you need to change the image anyway, you may consider reducing the space a little bit more between the bottom of your chin and the bottom of the picture (maintaining aspect ratio). The ideal image in this case would be to follow Nike Dattani's image on https://hpqc.org/people.html. Notice that the space is almost zero there! I can approve this PR ASAP after you commit these changes. |
@AbdulMuhaymin Please let me know if these changes are okay now. Thanks |
No description provided.