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

Jump to bottom

[new] Adding new library, sql-formatter via package.json #12893

Merged
merged 3 commits into from Mar 20, 2020
Merged

[new] Adding new library, sql-formatter via package.json #12893

merged 3 commits into from Mar 20, 2020

Conversation

ambischof
Copy link
Contributor

Pull request for issue: #12740
Related issue(s): N/A

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

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
  • [x ] This lib was not found on cdnjs repo
  • [x ] Doesn't already exist / has duplicate issue and PR
  • [ x] The lib has notable popularity
    • More than 200 [Stars / Watchers / Forks] on [GitHub / Bitbucket]
    • [ x] More than 800 downloads stats per month on npm registry
  • [x ] 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; clean, clear and easy to understand.
  • [ x] The parent of the commit(s) in the PR is not older than 3 days.
  • [x ] Pull request is sent from a non-master branch with a meaningful name.
  • [ x] Separate unrelated changes into different commits.
  • [ x] Use rebase to squash/fixup dummy/unnecessary commits into only one commit.
  • Close corresponding issue in commit message
  • Mention related issue(s), people in commit message, comment.

Adding new library, sql-formatter via package.json
18adf5b 
Added the library, sql-formatter, using package.json.
PeterBot
PeterBot suggested changes Jul 26, 2018
View reviewed changes
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 😨 18adf5b CI test failed ❗

@ambischof please take a look at CI build https://ci.cdnjs.com/cdnjs/cdnjs/22264 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 ☺️

extend1994
extend1994 reviewed Jul 30, 2018
View reviewed changes
Copy link
Contributor

@extend1994 extend1994 left a comment •
edited

Choose a reason for hiding this comment

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

Please checkout #12872 "Via single package.json" part and remove uneccessary fields.

@ambischof
Copy link
Contributor Author

@extend1994 I think I need some advice here. Im having difficulty cloning the repo locally due to size and so I don't know how I can fix it without making an extra commit. I don't think I can amend commits on github. I tried the sparse commit as written in the guide, but it still was downloading well over 2GB and saying it was like 10% done. Other guides I found also didn't help with that.

@extend1994
Copy link
Contributor

extend1994 commented Aug 2, 2018 •
edited

@ambischof extra commits are fine, I can help you squash them.
Making sure that all the fields are all cdnjs need is enough. :)

sufuf3
sufuf3 reviewed Sep 19, 2018
View reviewed changes
Copy link
Contributor

@sufuf3 sufuf3 left a comment

Choose a reason for hiding this comment

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

@ambischof Thank you for your contribution.
You did a nice work.
However, this PR need to be modified to make better be merged.
Would you please refer to #12950 & #12149?
There are some tips may help you revise this PR.
If you have some questions, please let me know.
Thank you.

ajax/libs/sql-formatter/package.json Outdated
"src"
],
"scripts": {
"clean": "rimraf lib dist",
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's remove scripts part.
Because CDNJS doesn't need this part.
Thanks

ajax/libs/sql-formatter/package.json Outdated
},
"bugs": {
"url": "https://github.com/zeroturnaround/sql-formatter/issues"
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's remove bugs part.
Because CDNJS doesn't need this part.
Thanks

ajax/libs/sql-formatter/package.json Outdated
},
"dependencies": {
"babel-runtime": "^6.18.0",
"lodash": "^4.16.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's remove dependencies part.
Because CDNJS doesn't need this part.
Thanks

ajax/libs/sql-formatter/package.json Outdated
},
"devDependencies": {
"babel-cli": "^6.14.0",
"babel-core": "^6.11.4",
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's remove devDependencies part.
Because CDNJS doesn't need this part.
Thanks

ajax/libs/sql-formatter/package.json Outdated
"webpack": "^1.13.1"
},
"jest": {
"testPathDirs": [
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's remove jest part.
Because CDNJS doesn't need this part.
Thanks

ajax/libs/sql-formatter/package.json Outdated
"version": "2.3.1",
"description": "Formats whitespaces in a SQL query to make it more readable",
"license": "MIT",
"main": "dist/sql-formatter.min.js",
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's modify as "filename": "sql-formatter.min.js",
This is the main file of this library, you might be able to find it after reading its documentation.
Thanks

ajax/libs/sql-formatter/package.json Outdated
],
"files": [
"dist",
"lib",
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's remove files part.
Because CDNJS doesn't need this part.
Thanks

ajax/libs/sql-formatter/package.json Outdated
@@ -0,0 +1,75 @@
{
"name": "sql-formatter",
"version": "2.3.1",
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's remove version field.
Because we add this lib via single package.json.
If you add version field, this means that you have to add version folder & add files from upstream under the version folder.
This may a little be hard for the beginner.
Thanks

MattIPv4
MattIPv4 approved these changes Mar 20, 2020
View reviewed changes
Copy link
Member

@MattIPv4 MattIPv4 left a comment

Choose a reason for hiding this comment

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

Lgtm.

@MattIPv4 MattIPv4 merged commit 1eae038 into cdnjs:master Mar 20, 2020
@ambischof ambischof deleted the sql-formatter branch March 20, 2020 13:11
robocdnjs added a commit to cdnjs/packages that referenced this pull request Mar 22, 2020
@robocdnjs
move sql-formatter
4f889ac 
cdnjs/cdnjs#12893
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sufuf3 sufuf3 sufuf3 left review comments

@extend1994 extend1994 extend1994 left review comments

@PeterBot PeterBot PeterBot requested changes

@MattIPv4 MattIPv4 MattIPv4 approved these changes

@dakshshah96 dakshshah96 Awaiting requested review from dakshshah96

@sashberd sashberd Awaiting requested review from sashberd

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@ambischof @extend1994 @sufuf3 @MattIPv4 @PeterBot