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

add vue-i18n@4.0.1 w/ npm autoupdate #8150

Merged
merged 1 commit into from
Jun 8, 2016

Conversation

pvnr0082t
Copy link
Contributor

PR for #7894
@kennynaoh Please help me review this pull request, thank you.
Here is an issue discuss with author about files to get in dist folder.

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/kazupon/vue-i18n
  • Official website (optional, not the repository):
  • NPM package url (optional): https://www.npmjs.com/package/vue-i18n
  • GitHub / Bitbucket popularity (required):
    • Count of stars: 216
    • Count of watchers: 17
    • Count of forks: 10
  • NPM download stats (optional):
    • Downloads in the last day: 50
    • Downloads in the last week: 715
    • Downloads in the last month: 2712

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.

close #7894, cc @kazupon

@kennynaoh
Copy link
Contributor

@pvnr0082t
Sorry for late review, please rebase to the latest master branch.

@pvnr0082t
Copy link
Contributor Author

@kennynaoh I have rebased on the latest master branch. Please help me review again, thank you.

@pvnr0082t pvnr0082t assigned kennynaoh and unassigned pvnr0082t Jun 6, 2016
@kennynaoh
Copy link
Contributor

@PeterDaveHello
I think this PR is correct.
Please help me review it again.
Thanks.

@kennynaoh kennynaoh assigned PeterDaveHello and unassigned kennynaoh Jun 6, 2016
@PeterDaveHello
Copy link
Contributor

What's the reason why to drop vue-i18n.common.js? You didn't mention any reason.

@kennynaoh
Copy link
Contributor

@PeterDaveHello
I think the issue author talked about is the reason.
Please help me take a look again.
Thanks!

@kennynaoh kennynaoh assigned PeterDaveHello and unassigned kennynaoh Jun 7, 2016
@PeterDaveHello
Copy link
Contributor

I didn't see any comment to mention the reason.

@pvnr0082t
Copy link
Contributor Author

@PeterDaveHello Author said that there is vue-i18n.min.js as minified file. I asked if vue-i18n.common.js don't need to be added. I think the reply by author is yes but I don't know the real reason.

@PeterDaveHello
Copy link
Contributor

@pvnr0082t You asked about the minified common file, not the origin one. Anyway, please confirm that, I did not see a reason of not hosting it, since it's in dist folder, it's odd not to host it.

@pvnr0082t pvnr0082t changed the title add vue-i18n v4.0.0 with npm autoupdate config add vue-i18n v4.0.1 with npm autoupdate config Jun 7, 2016
@pvnr0082t
Copy link
Contributor Author

@kennynaoh I have confirm with author again kazupon/vue-i18n#34. And the latest version is v4.0.1. Please help me review again, thank you.

@kennynaoh
Copy link
Contributor

@PeterDaveHello
I think this PR is correct.
Please help me review it again.
Thanks.

@PeterDaveHello
Copy link
Contributor

What's the reqason why not to minify vue-i18n.common.js? @pvnr0082t @kennynaoh

@kennynaoh
Copy link
Contributor

I thought the author had said that we don't need to minify vue-i18n.common.js, didn't it?
Actually, I am not pretty sure.

@PeterDaveHello
Copy link
Contributor

@kennynaoh We should minify it.

@pvnr0082t
Copy link
Contributor Author

@kennynaoh I have minified vue-i18n.common.js. Please help me review again, thank you.

@kennynaoh
Copy link
Contributor

@PeterDaveHello
I think this PR is correct.
Please help me review it again.
Thanks.

@PeterDaveHello PeterDaveHello changed the title add vue-i18n v4.0.1 with npm autoupdate config add vue-i18n@4.0.1 w/ npm autoupdate Jun 8, 2016
@PeterDaveHello
Copy link
Contributor

@pvnr0082t @kennynaoh please use a shorter title/first line commit next time, I'll modify the commit this time.

@PeterDaveHello PeterDaveHello merged commit a6bd620 into cdnjs:master Jun 8, 2016
@pvnr0082t pvnr0082t deleted the vue-i18n branch July 18, 2016 05:10
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.

[Request] Add vue-i18n
3 participants