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

Copy-edit README.md Issue: #11168 #12043

Merged
merged 1 commit into from Nov 6, 2017

Conversation

LeaSak
Copy link
Contributor

@LeaSak LeaSak commented Oct 24, 2017

Hello. I've copy-edited the README.md. These are just suggestions. Besides correcting some grammar and punctuation, I tried to reduce the length of some sentences and the use of passive to help improve readability. I also fixed the link to the API docs.

Pull request for issue: #11168
Related issue(s): # #

Checklist for Pull request or lib adding request issue follows the conventions.

Note that if you are using a distribution purpose repository/package, please also provide the url and other related info like popularity of the source code repo/package.

Profile of the lib

  • Git repository (required):
  • Official website (optional, not the repository):
  • NPM package url (optional):
  • License and its reference:
  • GitHub / Bitbucket popularity (required):
    • Count of watchers:
    • Count of stars:
    • Count of forks:
  • NPM download stats (optional):
    • Downloads in the last day:
    • Downloads in the last week:
    • Downloads in the last month:

Essential checklist

  • I'm the author of this library
    • I would like to add link to the page of this library on CDNJS on website / readme
  • This lib was not found on cdnjs repo
  • No already exist / duplicated issue and PR
  • The lib has notable popularity
    • More than 200 [Stars / Watchers / Forks] on [GitHub / Bitbucket]
    • More than 800 downloads stats per month on npm registry
  • Project has public repository on famous online hosting platform (or been hosted on npm)

Auto-update checklist

  • Has valid tags for each versions (for git auto-update)
  • Auto-update setup
  • Auto-update target/source is valid.
  • Auto-update filemap is correct.

Git commit checklist

  • [x ] The first line of commit message is less then 50 chars, be clean and clear, easy to understand.
  • [x ] The parent of the commit(s) in the PR is not old than 3 days.
  • [ x] Pull request is sending from a non-master branch with meaningful name.
  • Separate unrelated changes into different commits.
  • Use rebase to squash/fixup dummy/unnecessary commits into only one commit.
  • Close corresponding issue in commit message
  • [ X] Mention related issue(s), people in commit message, comment.

Copy link
Contributor

@PeterBot PeterBot left a comment

Choose a reason for hiding this comment

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

@LeaSak congratulations! 3393ce2 CI test passed! ✅
Please wait for the further review from the maintainers!

For the details 📃, please take a look at ➡️ https://ci.cdnjs.com/cdnjs/cdnjs/14238, thank you 😀

@maruilian11
Copy link
Contributor

maruilian11 commented Oct 26, 2017

Great! Thank you so much.

@maruilian11
Copy link
Contributor

maruilian11 commented Oct 26, 2017

@LeaSak Could you modify the issue number in commit msg to #11168? Then, I think everything is wonderful. 😄

Copy link
Contributor

@PeterBot PeterBot left a comment

Choose a reason for hiding this comment

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

Oops 😨 d3b8445 CI test failed ❗

@LeaSak please take a look at CI build https://ci.cdnjs.com/cdnjs/cdnjs/14375 for details 📝 !
Most of the error will have corresponding explanation, so that you will know what's wrong and then try to fix it!
If you cannot understand the error message and need help, feel free to ask our maintainers ☺️

Copy link
Contributor

@PeterBot PeterBot left a comment

Choose a reason for hiding this comment

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

@LeaSak congratulations! 6165426 CI test passed! ✅
Please wait for the further review from the maintainers!

For the details 📃, please take a look at ➡️ https://ci.cdnjs.com/cdnjs/cdnjs/14378, thank you 😀

@LeaSak
Copy link
Contributor Author

LeaSak commented Oct 26, 2017

Hi @maruilian11. I managed to modify the commit message and learn a bit more about git in the meantime 😃

Copy link
Contributor

@PeterBot PeterBot left a comment

Choose a reason for hiding this comment

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

@LeaSak congratulations! c7943ba CI test passed! ✅
Please wait for the further review from the maintainers!

For the details 📃, please take a look at ➡️ https://ci.cdnjs.com/cdnjs/cdnjs/14472, thank you 😀

Copy link
Contributor

@maruilian11 maruilian11 left a comment

Choose a reason for hiding this comment

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

I think this PR is great.

README.md Outdated

CDNJS is a free and open source project to organize all the famous web front-end development resources and provide them to the developers with faster CDN infrastructure without usage limitation and fee. We want to help individual library/framework developers spread their projects, and help web developers to supercharge their websites! With the great and free CDN service, developers can focus on the project and website development, without spending time worrying about how to setup a CDN for the project or website assets. We hope to make web development easier, your websites and the WWW faster and safer.
CDNJS is a free open-source project to organize and provide all famous front-end web development resources to developers via a fast CDN infrastructure without usage limitations and fees. We want to help individual library/framework developers distribute their projects, and web developers supercharge their websites! With our great free CDN service, developers can focus on their projects and website development. Developers no longer have to spend time worrying about how to set-up a CDN for projects or website assets. We hope to make web development easier, as well as your websites and the WWW faster and safer.
Copy link
Contributor

Choose a reason for hiding this comment

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

hmmm, I suppose we usually use the common term free and open source project instead of free open-source project?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @PeterDaveHello. Sure, I can fix that if you like. Also, in my PR I capitalized the acronym url to URL in all cases in the README body except for in the Table of Contents. The comments mentioned not to touch this section manually. Should I keep this change or switch it back to how it was beforehand to be consistent with the Table of Contents item?

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, let's change it back!

It's fine if you'd like to update the Table of Contents :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, great. Just one last question from a beginner. Should I squash the new commit with the above changes with my first commit on my branch after I make my changes?

Copy link
Contributor

Choose a reason for hiding this comment

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

@LeaSak Yes, we hope the commits can be as clean as possible. Could you please squash them? Thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @maruilian11 for helping me. I just squashed the commits.

Copy link
Contributor

@petrgazarov petrgazarov left a comment

Choose a reason for hiding this comment

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

Good to see this cleaned up! One comment below, otherwise LGTM

README.md Outdated

CDNJS is a free and open source project to organize all the famous web front-end development resources and provide them to the developers with faster CDN infrastructure without usage limitation and fee. We want to help individual library/framework developers spread their projects, and help web developers to supercharge their websites! With the great and free CDN service, developers can focus on the project and website development, without spending time worrying about how to setup a CDN for the project or website assets. We hope to make web development easier, your websites and the WWW faster and safer.
CDNJS is a free open-source project to organize and provide all famous front-end web development resources to developers via a fast CDN infrastructure without usage limitations and fees. We want to help individual library/framework developers distribute their projects, and web developers supercharge their websites! With our great free CDN service, developers can focus on their projects and website development. Developers no longer have to spend time worrying about how to set-up a CDN for projects or website assets. We hope to make web development easier, as well as your websites and the WWW faster and safer.
Copy link
Contributor

Choose a reason for hiding this comment

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

and provide all famous front-end web development resources to developers

all famous -> popular

Actors and celebrities can be described as "famous", but with software libraries it sounds a bit comical

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree. I'll swap the word. No problem.

Copy link
Contributor

@PeterBot PeterBot left a comment

Choose a reason for hiding this comment

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

@LeaSak congratulations! 632859f CI test passed! ✅
Please wait for the further review from the maintainers!

For the details 📃, please take a look at ➡️ https://ci.cdnjs.com/cdnjs/cdnjs/14636, thank you 😀

Copy link
Contributor

@PeterBot PeterBot left a comment

Choose a reason for hiding this comment

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

@LeaSak congratulations! fef0c65 CI test passed! ✅
Please wait for the further review from the maintainers!

For the details 📃, please take a look at ➡️ https://ci.cdnjs.com/cdnjs/cdnjs/14637, thank you 😀

Copy link
Contributor

@PeterBot PeterBot left a comment

Choose a reason for hiding this comment

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

@LeaSak congratulations! 6741080 CI test passed! ✅
Please wait for the further review from the maintainers!

For the details 📃, please take a look at ➡️ https://ci.cdnjs.com/cdnjs/cdnjs/14757, thank you 😀

Copy link
Contributor

@dakshshah96 dakshshah96 left a comment

Choose a reason for hiding this comment

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

LGTM


**Working on your first Pull Request?** You can learn how from this *free* series [How to Contribute to an Open Source Project on GitHub](https://egghead.io/series/how-to-contribute-to-an-open-source-project-on-github)
**Working on your first Pull Request?** Learn how from this *free* series: [How to Contribute to an Open Source Project on GitHub](https://egghead.io/series/how-to-contribute-to-an-open-source-project-on-github)
Copy link
Contributor

Choose a reason for hiding this comment

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

This was copied from http://makeapullrequest.com/ actually, but I won't mind if we have better wording.

Copy link
Contributor

@PeterDaveHello PeterDaveHello left a comment

Choose a reason for hiding this comment

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

Awesome 👍

@PeterDaveHello PeterDaveHello merged commit 93720e8 into cdnjs:master Nov 6, 2017
@ghost ghost removed the in progress label Nov 6, 2017
@LeaSak
Copy link
Contributor Author

LeaSak commented Nov 6, 2017

Thanks for everyone's help and patience. This is exciting.

@PeterDaveHello
Copy link
Contributor

@LeaSak Thank you for your contribution 😄

@LeaSak LeaSak deleted the edit-readme-#11168 branch November 18, 2017 12:16
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.

None yet

6 participants