Skip to content
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

more designs added #4025

Merged
merged 2 commits into from
Mar 21, 2019
Merged

more designs added #4025

merged 2 commits into from
Mar 21, 2019

Conversation

mehul0698
Copy link
Contributor

@mehul0698 mehul0698 commented Mar 21, 2019

Description
Checklist
Affected core subsystem(s)
Refers/Fixes
Testing and Sign-off
Contributor
  • Read and followed the Contributor Guidelines
  • Tested all changes locally
  • Verified existing functionality
  • Ran make test and everything passed!
Reviewer
  • Affirm contributor guidelines have been followed and requested changes made
  • CI tests and linting pass
  • No conflicts (migrations, files, etc)
  • Regression tested against staging or local deployment
Funder
  • Validated requested changes were made to specification
  • Bounty payout released to the contributor

@mehul0698
Copy link
Contributor Author

image
image
image
image
image
image
image
image
image
image
image
image
image
image

@mehul0698
Copy link
Contributor Author

@owocki @thelostone-mc I have added 7 more accessories. The last one is a sword. It looks very cool, do check that out. Please review the pr.

@codecov
Copy link

codecov bot commented Mar 21, 2019

Codecov Report

Merging #4025 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #4025   +/-   ##
=======================================
  Coverage   30.07%   30.07%           
=======================================
  Files         205      205           
  Lines       16006    16006           
  Branches     2111     2111           
=======================================
  Hits         4814     4814           
  Misses      11020    11020           
  Partials      172      172
Impacted Files Coverage Δ
app/avatar/utils.py 13.43% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9c87441...e6aa79c. Read the comment docs.

@codecov
Copy link

codecov bot commented Mar 21, 2019

Codecov Report

Merging #4025 into master will decrease coverage by 12.9%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #4025       +/-   ##
===========================================
- Coverage   30.07%   17.16%   -12.91%     
===========================================
  Files         205      194       -11     
  Lines       16000    15920       -80     
  Branches     2109     2107        -2     
===========================================
- Hits         4812     2733     -2079     
- Misses      11017    13178     +2161     
+ Partials      171        9      -162
Impacted Files Coverage Δ
app/avatar/utils.py 13.43% <ø> (ø) ⬆️
app/kudos/forms.py 0% <0%> (-100%) ⬇️
app/grants/serializers.py 0% <0%> (-100%) ⬇️
app/avatar/serializers.py 0% <0%> (-100%) ⬇️
app/grants/urls.py 0% <0%> (-100%) ⬇️
app/inbox/urls.py 0% <0%> (-100%) ⬇️
app/avatar/urls.py 0% <0%> (-100%) ⬇️
app/grants/forms.py 0% <0%> (-100%) ⬇️
app/avatar/router.py 0% <0%> (-90.91%) ⬇️
app/app/urls.py 0% <0%> (-90%) ⬇️
... and 78 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8cd6b4a...da0edec. Read the comment docs.

thelostone-mc
thelostone-mc previously approved these changes Mar 21, 2019
@mehul0698
Copy link
Contributor Author

Hey did you see the pictures I posted on the other thread with all the misplaced assets?

@thelostone-mc
Copy link
Member

@mehul0698 mind tagging the link here ?

@mehul0698
Copy link
Contributor Author

@thelostone-mc #3971

@mehul0698
Copy link
Contributor Author

@thelostone-mc don't merge this pr. I'll push the corrected assets on this branch.

@mehul0698
Copy link
Contributor Author

More mistakes
image

@mehul0698
Copy link
Contributor Author

image
image
image
image
image
image

@mehul0698
Copy link
Contributor Author

@thelostone-mc I have corrected 6 assets. Please review the pr.

Copy link
Member

@thelostone-mc thelostone-mc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah man you da best ❤️ Thanks for fixing those up
cc @owocki

@mehul0698
Copy link
Contributor Author

Will I get paid for correcting them?

@thelostone-mc thelostone-mc merged commit b0ab176 into gitcoinco:master Mar 21, 2019
@owocki
Copy link
Contributor

owocki commented Mar 21, 2019

hey -- before i answer that question @mehul0698 -- who originally added the assets that were misplaced? does your fix apply across all assets / how can we prevent this in the future? how did you fix them and how much time did it take per asset

@mehul0698
Copy link
Contributor Author

mehul0698 commented Mar 21, 2019

@owocki I think the assets were added by yash412. I have fixed the ones that I was able to find. To prevent in this future you just ask them to upload 2 pictures for each asset one in avatar builder and one once it is saved as the problem with these assets was not in the avatar builder but once they were saved they appeared out of place. 4 of these I had to resize and make minor changes and change their .txt file but the bird and necklace I made them from scratch as resizing was not working for them. For all the assets that I make upload 2 pictures to give you the assurance that they will work anywhere and that they meet size criteria. The problem with some of these assets was that the artboard size was messed up and once I corrected that the asset got messed up so had to redo a lot in it. I hope this helps.

@mehul0698
Copy link
Contributor Author

@owocki I think there might be more as when I was looking for inspiration I found these.

@owocki
Copy link
Contributor

owocki commented Mar 21, 2019

@mehul0698 did you correct the artboard sizes in sketch? if so, are there guidelines with respect to artboard sizes we should start enforcing?

@mehul0698
Copy link
Contributor Author

Yes and do make sure they upload both pictures as it might look alright in the avatar builder but once saved it appears out of place. In the bounty description you have mentioned the gitcoin style guide in one line. I suggest you put more emphasis on that as some designs dont even follow the colour guidelines. Mainly I saw some developer using a misplaced asset on gitcoin and does not look good for the brand.

@mehul0698
Copy link
Contributor Author

@owocki the mail you sent about avatars to everyone also had misplaced assets. The one I got was also in bad shape.

@mehul0698
Copy link
Contributor Author

image
Look at this one.

@mehul0698
Copy link
Contributor Author

@owocki The black partial background ruins this clothing option. No one is going to use this.

@mehul0698
Copy link
Contributor Author

image
In the avatar builder this looks fine.

@mehul0698
Copy link
Contributor Author

mehul0698 commented Mar 21, 2019

image
@owocki This one also.

@mehul0698
Copy link
Contributor Author

@owocki The pr has been merged. I made 7 accessories out of which I consider 4 to be exotic. You owe me around 125 to 130 dollars.
If its fine by you I corrected 6 accessories out of which 2 I had to make from scratch, for that you could pay me 50 to 60 dollars.
If you want I could look into other messed up assets also. TIA

@mehul0698
Copy link
Contributor Author

@owocki do you want me to look into other assets?

@mehul0698
Copy link
Contributor Author

@owocki @thelostone-mc I have made 2 more designs should put a pr?

@mehul0698 mehul0698 deleted the more_assets branch March 25, 2019 13:19
@owocki
Copy link
Contributor

owocki commented Mar 25, 2019

sorry, was away for the weekend. yes pls PR the last two designs

@mehul0698
Copy link
Contributor Author

@owocki As avengers endgame is about to release so one of them is avengers related. If it contains the avengers logo will it have any proprietary problems?

@owocki
Copy link
Contributor

owocki commented Mar 25, 2019

its probably not a great idea to include anything proprietary. it can be inspired by avengers, but not lifted from avengers

@mehul0698
Copy link
Contributor Author

Cool. I'll change it.

@owocki
Copy link
Contributor

owocki commented Mar 26, 2019

@mehul0698 hey just got back from traveling. whats our tab up to now? i see i paid you 1.1 ETH about 6 days ago

@mehul0698
Copy link
Contributor Author

All previous payments are done except this pr. This was merged 5 days ago.
I made 7 accessories out of which I consider 4 to be exotic. You owe me around 125 to 130 dollars.
If its fine by you I corrected 6 accessories out of which 2 I had to make from scratch, for that you could pay me 50 to 60 dollars.
I have put another pr with 3 designs #4040

@owocki
Copy link
Contributor

owocki commented Mar 26, 2019

ok so 175 to 190?

@mehul0698
Copy link
Contributor Author

Yes. Thanks

@gitcoinbot
Copy link
Member

⚡️ A tip worth 1.35000 ETH (180.1 USD @ $133.41/ETH) has been granted to @mehul0698 for this issue from @owocki. ⚡️

Nice work @mehul0698! Your tip has automatically been deposited in the ETH address we have on file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants