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

Uppercase labels too #380

Merged
merged 2 commits into from
Oct 4, 2017
Merged

Uppercase labels too #380

merged 2 commits into from
Oct 4, 2017

Conversation

skjnldsv
Copy link
Member

@skjnldsv skjnldsv commented Oct 4, 2017

Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
@skjnldsv skjnldsv added 3. to review Waiting for reviews bug Something isn't working compatibility Compatibility with other services Hacktoberfest good first issue Good for newcomers labels Oct 4, 2017
@skjnldsv skjnldsv added this to the 2.0.2 milestone Oct 4, 2017
@skjnldsv skjnldsv self-assigned this Oct 4, 2017
@skjnldsv skjnldsv changed the title To uppercase for labels too Uppercase labels too Oct 4, 2017
@codecov
Copy link

codecov bot commented Oct 4, 2017

Codecov Report

Merging #380 into master will not change coverage.
The diff coverage is 0%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master    #380   +/-   ##
======================================
  Coverage    14.2%   14.2%           
======================================
  Files          55      55           
  Lines        1267    1267           
======================================
  Hits          180     180           
  Misses       1087    1087
Impacted Files Coverage Δ
...s/components/detailsItem/detailsItem_controller.js 1.56% <0%> (ø) ⬆️

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 08ca404...08e4916. Read the comment docs.

Copy link
Member

@xh3n1 xh3n1 left a comment

Choose a reason for hiding this comment

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

Hey @skjnldsv , I don't think it's a good idea to keep console statements.

@skjnldsv
Copy link
Member Author

skjnldsv commented Oct 4, 2017

@xh3n1 I don't think either. I'm an idiot! 😂

Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
@skjnldsv
Copy link
Member Author

skjnldsv commented Oct 4, 2017

@xh3n1 ok now!

Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

Fully capitalizing looks veeery screamy and out of place. ;) We should just use »Home«, »Work«, »Mobile«, »Work pager«, »Telex«, »Home fax«, »Fax« etc.

@skjnldsv
Copy link
Member Author

skjnldsv commented Oct 4, 2017

@jancborchardt did you test the pr? ^^'

@jancborchardt
Copy link
Member

@skjnldsv sorry, I looked at the comment in #67 (comment) and assumed that’s how it’s going to be displayed. ;)

@skjnldsv
Copy link
Member Author

skjnldsv commented Oct 4, 2017

@jancborchardt Don't worry, made me laugh! 😝 ❣️

@jancborchardt
Copy link
Member

I'm happy to entertain :D

@skjnldsv skjnldsv merged commit ad9d0d3 into master Oct 4, 2017
@skjnldsv skjnldsv deleted the capitalize-props branch October 4, 2017 21:03
@skjnldsv skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Oct 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish bug Something isn't working compatibility Compatibility with other services good first issue Good for newcomers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants