-
Notifications
You must be signed in to change notification settings - Fork 380
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
docs: update screenshots in README #1017
Conversation
docs: update screenshots in README
the changes are visible here |
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.
@singhanurag05 could you please upload these images in a comment and then use instead the link of the URLs on the README? In this way, we can remove the binary files from the repository. Having these binary files adds unnecessary weight to this repository.
Here's a guide if you have any doubt on how to upload the images: https://gist.github.com/vinkla/dca76249ba6b73c5dd66a4e986df4c8d
I'll be doing this on the 7th of November as I have my mid semester exams starting tomorrow. Would that be fine? |
|
@singhanurag05 are you still going to work on this issue? It is ok if not, we just need to know for triaging issues and pull requests purposes |
I currently wont be able to complete this. Going through a rough patch in life. really sorry for holding stuff up for this long |
@singhanurag05 can you update the PR? |
yes sure! |
uploaded photos in a github comment (anitab-org#1017 (comment)) and added the image urls in the readme so that the screenshots folder could be removed from the repo.
@aditmehta9 @isabelcosta I have made the changes please check. |
@vj-codes can you review this? |
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.
LGTM
@singhanurag05 hey almost done but as mentioned above you will need to remove the binary files as well from the PR. |
Ohh yes I agree I didn't notice that. |
i cant figure out how to squash commits but I think someone who is able to merge the pr can squash the commits |
@aditmehta9 can you review this again please? |
Yes, sure, I just noticed that there isn't splash screen screenshot in this PR is it ok? Should I approve this PR? |
@aditmehta9 there's no splash screen currently for the apk |
Ok 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.
LGTM 🎉
@singhanurag05 thank you for making the changes:)
Adding label ready to merge since it's documentation change |
Tested after changes: bandicam.2021-02-17.03-25-41-216.mp4 |
i have changed my username to @heyanurag so please tag me there if there are further changes |
Description
Include a summary of the change and relevant motivation/context. List any dependencies that are required for this change.
Fixes #758
Type of Change:
Delete irrelevant options.
Code/Quality Assurance Only
How Has This Been Tested?
Describe the tests you ran to verify your changes. Provide instructions or GIFs so we can reproduce. List any relevant details for your test.
Checklist:
Delete irrelevant options.
Code/Quality Assurance Only