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

[new] Add choices.js v2.0.2 #8927

Closed
wants to merge 5 commits into from
Closed

[new] Add choices.js v2.0.2 #8927

wants to merge 5 commits into from

Conversation

iv-craig
Copy link

@iv-craig iv-craig commented Sep 6, 2016

First time I'm adding a package, so I'm not sure if I've got the auto-update/git commit checklists right. Please let me know if there's anything I need to update.

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
  • This lib was not found on cdnjs repo
  • No already exist / duplicated issue and PR
  • The lib has notable popularity
    • More than 100 [Stars / Watchers / Forks] on [GitHub / Bitbucket]
    • More than 500 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

  • The first line of commit message is less then 50 chars, be clean and clear, easy to understand.
  • The parent of the commit(s) in the PR is not old than 3 days.
  • 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
  • Mention related issue(s), people in commit message, comment.

@PeterDaveHello
Copy link
Contributor

@cdnjs/intern2 please help review this PR, thanks.

"name": "choices.js",
"version": "2.0.2",
"description": "A vanilla JS customisable text input/select box plugin",
"main": "./assets/scripts/dist/choices.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.

@iv-craig Let's remove some fields we don't need. Like main, scripts, bugs, devDependencies.

@kennynaoh
Copy link
Contributor

kennynaoh commented Sep 7, 2016

Hello @iv-craig ,
Please add filename filed, like "filename":"choices.min.js".
Thanks!

@kennynaoh
Copy link
Contributor

kennynaoh commented Sep 7, 2016

Hello @iv-craig
Maybe you can change the npmFileMap.

{
     "basePath": "assets/scripts/dist",
     "files": [
          "**/*"
      ]
},
{
     "basePath": "assets/styles",
     "files": [
           "css/+(base|choice)*"
     ]
},
{
       "basePath": "assets/icons",
       "files": [
            "*.svg"
      ]
}

Thanks!

@iv-craig
Copy link
Author

iv-craig commented Sep 7, 2016

Hi @kennynaoh
I ran the package.json through the fixFormat.js tool, but CircleCI is still showing an error with the formatting. Does anything look off there? Thanks!

@PeterDaveHello
Copy link
Contributor

@iv-craig Did you run under the root of cdnjs by ./tools/fixFormat.js?

@iv-craig
Copy link
Author

iv-craig commented Sep 7, 2016

@PeterDaveHello I had put the fixFormat functionality into a gulpfile and ran that locally to avoid cloning the whole cdnjs repo, but even after cloning the repo and running it from the cdnjs root it doesn't appear to have made any changes.

@PeterDaveHello
Copy link
Contributor

@iv-craig any error there? Can you try to run it via command line instead of via gulp? Thanks.

@PeterDaveHello
Copy link
Contributor

@kennynaoh the filemap looks a little bit complex, could it be easier and cleaner?
@cdnjs/intern2 can you give other advices? Thanks.

@pvnr0082t
Copy link
Contributor

It can pass the test after I ran tools/fixFormat.js. But I diff the package.json before and after I ran tools/fixFormat.js, they are totally the same.

@iv-craig
Copy link
Author

iv-craig commented Sep 7, 2016

@PeterDaveHello @pvnr0082t Committing from the command-line instead of the browser seems to have worked -- doing a sparse checkout also helped, I was getting errors during an earlier clone attempt. Thanks!

@PeterDaveHello
Copy link
Contributor

@iv-craig Congratulations!

@PeterDaveHello
Copy link
Contributor

@cdnjs/intern2 any updates here?

@x09326
Copy link
Contributor

x09326 commented Sep 8, 2016

@iv-craig
Please add keywords field in package.json.

@PeterDaveHello
Copy link
Contributor

@kennynaoh the filemap looks a little bit complex, could it be easier and cleaner?

@x09326
Copy link
Contributor

x09326 commented Sep 9, 2016

@PeterDaveHello
How about this?

{
     "basePath": "assets",
     "files": [
          "scripts/dist/**/*",
          "styles/css/+(base|choice)*",
          "icons/*.svg"
      ]
}

@PeterDaveHello
Copy link
Contributor

We may need to slow down this PR, the structure looks not so good and common, can somebody help discuss with the author about the possibility to distribute all the distribution purpose files under a single dist? Thanks.

@PeterDaveHello
Copy link
Contributor

Any updates here? Thanks!

@PeterBot
Copy link
Contributor

PeterBot commented Apr 4, 2017

Any updates here? Thanks!

@iv-craig
Copy link
Author

iv-craig commented Apr 4, 2017

No updates, sorry. Been under a time crunch for...a while, and it's not letting up any time soon. Feel free to close this if you'd like.

@PeterDaveHello
Copy link
Contributor

@iv-craig no worries, I'll take care of it.

@PeterBot
Copy link
Contributor

Any updates here? Thanks!

@sufuf3 sufuf3 self-assigned this Jun 30, 2017
maruilian11 pushed a commit to sufuf3/cdnjs that referenced this pull request Aug 14, 2017
maruilian11 pushed a commit to sufuf3/cdnjs that referenced this pull request Aug 14, 2017
Manually add v2.7.3, v2.7.4, v2.7.5, v2.8.1, v2.8.2 from GitHub due to
their unavailability on npm. cc cdnjs#8927, cdnjs#11170, cdnjs#11540
@ghost ghost removed the in progress label Aug 15, 2017
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

8 participants