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.

Sign up for GitHub

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

Jump to bottom

[bundlephobia] add badge for the npm package size #1481

Merged
merged 17 commits into from
Feb 3, 2018
Merged

[bundlephobia] add badge for the npm package size #1481

merged 17 commits into from
Feb 3, 2018

Conversation

tlaziuk
Copy link
Contributor

@tlaziuk tlaziuk commented Feb 1, 2018 •
edited
Loading

the old PR - #1391

todo

  • merge master
  • refactor tests
  • address comments

@shields-ci
Copy link

shields-ci commented Feb 1, 2018 •
edited
Loading

Messages
📖

✨ Thanks for your contribution to Shields, @tlaziuk!
📖

Thanks for contributing to our documentation. We ❤️ our documentarians!

Generated by 🚫 dangerJS

@RedSparr0w
Copy link
Member

Hey @tlaziuk if you could please merge master again the tests should run as expected 😄

@tlaziuk tlaziuk changed the title [bundlephobia] [WiP] add badge for the npm package size [bundlephobia] add badge for the npm package size Feb 2, 2018
@tlaziuk
Copy link
Contributor Author

tlaziuk commented Feb 2, 2018

hey maintainers, you have to do something with the tests - take a look at the results - https://circleci.com/gh/badges/shields/1917?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link

@tlaziuk
Copy link
Contributor Author

tlaziuk commented Feb 2, 2018

I've refactored the test in 7bd0d9f IMO it's more readable now and covers the same cases as the previous code

@paulmelnikow paulmelnikow mentioned this pull request Feb 2, 2018
Closed
@paulmelnikow paulmelnikow added the service-badge New or updated service badge label Feb 2, 2018
paulmelnikow
paulmelnikow reviewed Feb 2, 2018
View reviewed changes
server.js Outdated
};

const formatErrorCode = (code) =>
code.replace(/([A-Z])/g, ' $1').substring(1).toLowerCase();
Copy link
Member

Choose a reason for hiding this comment

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

How about .trim() instead of .substring(1), which is clearer and possibly a little more resilient to changes in the input?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree with you.

service-tests/bundlephobia.js
t.create(`Testing format '${format}' against '${get}'`)
.get(get)
.expectJSONTypes(Joi.object().keys(expect))
}
Copy link
Member

Choose a reason for hiding this comment

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

Lovely, much obliged!

@paulmelnikow paulmelnikow merged commit 8ad176d into badges:master Feb 3, 2018
@paulmelnikow
Copy link
Member

Thanks for the teamwork!

@paulmelnikow paulmelnikow mentioned this pull request Feb 3, 2018
Closed
@tizmagik
Copy link

tizmagik commented Feb 3, 2018

Yay! I've been watching/waiting on this badge 😁 Looking forward to release 🎉

@pastelsky
Copy link

pastelsky commented Feb 4, 2018 •
edited
Loading

@tlaziuk Great work! Thanks a lot for taking this forward.

@tizmagik
Copy link

tizmagik commented Feb 4, 2018

I did not do any work, it’s thanks to @tlaziuk! I’m just an eager bystander 😂

@tlaziuk tlaziuk deleted the bundlephobia branch February 4, 2018 10:49
@pastelsky
Copy link

@paulmelnikow Any idea when this is going to go live ?

@platan
Copy link
Member

platan commented Feb 12, 2018

@pastelsky, I will quote @paulmelnikow on that:

Deploys usually happen every 1–3 weeks. Thaddée, who has limited time on this project, is the only sysadmin. He's working on giving me access to deploy and logs, but doing so is complicated because the hosting account (and maybe the servers too) are shared with other services he runs.

@pastelsky
Copy link

Great! Looking forward to it.

@paulmelnikow paulmelnikow mentioned this pull request Mar 8, 2018
Closed
@frenzzy
Copy link

frenzzy commented Mar 14, 2018

Any news when this badge will be available?

@blakeembrey blakeembrey mentioned this pull request May 23, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@paulmelnikow paulmelnikow paulmelnikow left review comments

Assignees
No one assigned
Labels
service-badge New or updated service badge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@tlaziuk @shields-ci @RedSparr0w @paulmelnikow @tizmagik @pastelsky @platan @frenzzy