Skip to content

[New] Add Modaal@0.3.1 w/ git auto-update#11981

Merged
PeterDaveHello merged 1 commit into
cdnjs:masterfrom
tingwei628:cdnjs-issue11929-add-Modaal-package-v2
Oct 17, 2017
Merged

[New] Add Modaal@0.3.1 w/ git auto-update#11981
PeterDaveHello merged 1 commit into
cdnjs:masterfrom
tingwei628:cdnjs-issue11929-add-Modaal-package-v2

Conversation

@tingwei628

@tingwei628 tingwei628 commented Oct 11, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor

Pull request for issue: #11929
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): https://github.com/humaan/Modaal
  • Official website (optional, not the repository): https://humaan.com/modaal/
  • NPM package url (optional): modaal
  • License and its reference: MIT
  • GitHub / Bitbucket popularity (required):
    • Count of watchers: 62
    • Count of stars: 2405
    • Count of forks: 127
  • NPM download stats (optional):
    • Downloads in the last day: 62
    • Downloads in the last week: 230
    • Downloads in the last month: 627

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

  • 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.

@tingwei628 tingwei628 mentioned this pull request Oct 11, 2017
Closed
19 tasks
PeterBot
PeterBot suggested changes Oct 11, 2017
View reviewed changes

@PeterBot PeterBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Oops 😨 bd37070 CI test failed ❗

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

PeterBot
PeterBot suggested changes Oct 11, 2017
View reviewed changes

@PeterBot PeterBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Oops 😨 18f1a73 CI test failed ❗

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

PeterBot
PeterBot approved these changes Oct 11, 2017
View reviewed changes

@PeterBot PeterBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@tingwei628 congratulations! 85267b0 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/13673, thank you 😀

@tingwei628 tingwei628 mentioned this pull request Oct 11, 2017
Closed
19 tasks
@ghost ghost assigned maruilian11 Oct 12, 2017
@ghost ghost added the in progress label Oct 12, 2017
@maruilian11

maruilian11 commented Oct 12, 2017

Copy link
Copy Markdown
Contributor

@tingwei628 Awesome work! Could you also fulfill the init comment about the lib info to help other reviewers review your PR more quickly? Thanks.

@maruilian11 maruilian11 requested a review from a team October 12, 2017 18:52
PeterBot
PeterBot approved these changes Oct 12, 2017
View reviewed changes

@PeterBot PeterBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@tingwei628 congratulations! 9b8de69 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/13737, thank you 😀

@tingwei628

tingwei628 commented Oct 12, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@maruilian11
The lib info:
"Modaal is a WCAG 2.0 Level AA accessible modal window plugin"

The lib repo url here
https://github.com/humaan/Modaal/blob/master/README.md

@tingwei628

tingwei628 commented Oct 12, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

This PR for issue #11929

@sufuf3

sufuf3 commented Oct 13, 2017

Copy link
Copy Markdown
Contributor

@tingwei628
@maruilian11 's mean is that she want you to fill in Profile of the lib, Essential checklist, Auto-update checklist & Git commit checklist of #11981 (comment) this comment.
Except for this part, this PR is good to me.

Thank you.

@maruilian11

maruilian11 commented Oct 13, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor

@PeterDaveHello Since sufuf3 and I both approve this PR, I think I can request your review.

@PeterDaveHello

PeterDaveHello commented Oct 13, 2017

Copy link
Copy Markdown
Contributor

@maruilian11 thanks

@tingwei628

tingwei628 commented Oct 13, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@maruilian11 @sufuf3
profile filled!

sufuf3
sufuf3 approved these changes Oct 13, 2017
View reviewed changes

@sufuf3 sufuf3 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM
Nice work! 😃

@sufuf3

sufuf3 commented Oct 13, 2017

Copy link
Copy Markdown
Contributor

@maruilian11 @extend1994 please help review this PR, thanks.

@PeterDaveHello PeterDaveHello merged commit d459aa9 into cdnjs:master Oct 17, 2017
@ghost ghost removed the in progress label Oct 17, 2017
@PeterDaveHello

PeterDaveHello commented Oct 17, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor

@sufuf3 too careless to approve without check the hash of the files ...

PeterDaveHello added a commit that referenced this pull request Oct 17, 2017
@PeterDaveHello
Fix Modaal v0.3.1 asset integrity, cc #11981
ba75f13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dakshshah96 dakshshah96 Awaiting requested review from dakshshah96
@extend1994 extend1994 Awaiting requested review from extend1994
@maruilian11 maruilian11 Awaiting requested review from maruilian11
@pvnr0082t pvnr0082t Awaiting requested review from pvnr0082t
@PeterDaveHello PeterDaveHello Awaiting requested review from PeterDaveHello
2 more reviewers
@sufuf3 sufuf3 sufuf3 approved these changes
@PeterBot PeterBot PeterBot approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@maruilian11 maruilian11

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@tingwei628 @maruilian11 @sufuf3 @PeterDaveHello @PeterBot