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

Created Bunny Ears accessory #2494

Merged

Conversation

@jakerockland
Copy link
Contributor

jakerockland commented Oct 19, 2018

Description

Adds bunny ear accessory to avatar creator, re: #2333

Checklist
  • linter status: 100% pass
  • changes don't break existing behavior
  • commit message follows commit guidelines
Affected core subsystem(s)

avatar

Testing

Tested locally, screenshots attached:

screenshot 2018-10-19 01 35 12

screenshot 2018-10-19 01 36 26

Refers/Fixes

Refs: #2333

@@ -98,7 +98,7 @@ def get_avatar_context():
'options': (['Glasses-0'], ['Glasses-1'], ['Glasses-2'], ['Glasses-3'], ['Glasses-4'], [
'HatShort-backwardscap'
], ['HatShort-ballcap'], ['HatShort-cowboy'], ['HatShort-headphones'],
['HatShort-shortbeanie'], ['HatShort-tallbeanie'], ['Earring-0'], ['Earring-1'], [
['HatShort-shortbeanie'], ['HatShort-tallbeanie'], ['HatShort-bunnyears'], ['Earring-0'], ['Earring-1'], [

This comment has been minimized.

Copy link
@stickler-ci

stickler-ci Oct 19, 2018

E501 line too long (143 > 120 characters)

@jakerockland jakerockland mentioned this pull request Oct 19, 2018
0 of 5 tasks complete
@codecov

This comment has been minimized.

Copy link

codecov bot commented Oct 19, 2018

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2494   +/-   ##
=======================================
  Coverage   29.22%   29.22%           
=======================================
  Files         147      147           
  Lines       11872    11872           
  Branches     1605     1605           
=======================================
  Hits         3469     3469           
  Misses       8285     8285           
  Partials      118      118
Impacted Files Coverage Δ
app/avatar/utils.py 18.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 c25f3e0...e5581be. Read the comment docs.

Copy link
Member

thelostone-mc left a comment

Could we compress the svg using svgo. Otherwise LGTM ^_^

@jakerockland

This comment has been minimized.

Copy link
Contributor Author

jakerockland commented Oct 20, 2018

@codecov

This comment has been minimized.

Copy link

codecov bot commented Oct 22, 2018

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2494      +/-   ##
==========================================
- Coverage   29.22%   29.13%   -0.09%     
==========================================
  Files         147      148       +1     
  Lines       11872    11928      +56     
  Branches     1605     1616      +11     
==========================================
+ Hits         3469     3475       +6     
- Misses       8285     8335      +50     
  Partials      118      118
Impacted Files Coverage Δ
app/avatar/utils.py 18.43% <ø> (ø) ⬆️
app/retail/emails.py 19.76% <0%> (ø) ⬆️
...board/management/commands/cleanup_dupe_profiles.py 0% <0%> (ø)
app/dashboard/embed.py 31.6% <0%> (+3.44%) ⬆️

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 c25f3e0...6a4af0b. Read the comment docs.

@codecov

This comment has been minimized.

Copy link

codecov bot commented Oct 22, 2018

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2494      +/-   ##
==========================================
- Coverage   29.13%   29.08%   -0.06%     
==========================================
  Files         148      148              
  Lines       11928    11928              
  Branches     1616     1616              
==========================================
- Hits         3475     3469       -6     
- Misses       8335     8341       +6     
  Partials      118      118
Impacted Files Coverage Δ
app/avatar/utils.py 18.43% <ø> (ø) ⬆️
app/dashboard/embed.py 28.16% <0%> (-3.45%) ⬇️

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 d6e81dc...36ca0b3. Read the comment docs.

@jakerockland

This comment has been minimized.

Copy link
Contributor Author

jakerockland commented Oct 22, 2018

@thelostone-mc So I think I fixed things, but there is a CodeCov failure now, is that something I need to pay mind to for this PR?

@jakerockland

This comment has been minimized.

Copy link
Contributor Author

jakerockland commented Oct 22, 2018

Fixed merge conflict 👍

['HatShort-tallbeanie'], ['Earring-0'],
['Earring-1'], ['EarringBack-2', 'Earring-2'],
['HatShort-tallbeanie'], ['HatShort-bunnyears'],
['Earring-0'], ['Earring-1'],

This comment has been minimized.

Copy link
@stickler-ci

stickler-ci Oct 22, 2018

W291 trailing whitespace

This comment has been minimized.

Copy link
@thelostone-mc

thelostone-mc Oct 23, 2018

Member

@jakerockland could you address this too?
And the CodeCov failure we can ignore for this PR

Copy link
Member

thelostone-mc left a comment

@jakerockland never mind ^_^ fixed it up
Thanks :)

@thelostone-mc thelostone-mc merged commit 0aa991f into gitcoinco:master Oct 23, 2018
2 of 5 checks passed
2 of 5 checks passed
ci/circleci CircleCI is running your tests
Details
codebeat analysis in progress
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/jenkins/pr-merge This commit looks good
Details
stickler-ci No lint errors found.
@jakerockland

This comment has been minimized.

Copy link
Contributor Author

jakerockland commented Oct 23, 2018

Ah rad thank you for doing that @thelostone-mc

@jakerockland jakerockland deleted the jakerockland:bunny-ear-avatar-accessory branch Oct 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.