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 onecolor v3.0.2 with npm auto-update config #7878

Merged
merged 1 commit into from
May 16, 2016

Conversation

x09326
Copy link
Contributor

@x09326 x09326 commented May 11, 2016

PR for #5928
@pvnr0082t please help me review it, thanks.
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/One-com/one-color
  • Official website (optional, not the repository):
  • NPM package url (optional):https://www.npmjs.com/package/onecolor
  • GitHub / Bitbucket popularity (required):
    • Count of stars:395
    • Count of watchers:26
    • Count of forks:23
  • NPM download stats (optional):
    • Downloads in the last day:9953
    • Downloads in the last week:48535
    • Downloads in the last month:199759

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.

cc #5928, cc @Munter

{
"basePath": "",
"files": [
"one-color.*"
Copy link
Contributor

@pvnr0082t pvnr0082t May 12, 2016

Choose a reason for hiding this comment

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

This rule won't grab one-color-all.js and one-color-all.map

@pvnr0082t
Copy link
Contributor

@x09326 Both of one-color-all.js and one-color.js have already been minified and have their map file. You don't need to minify again.

@x09326
Copy link
Contributor Author

x09326 commented May 13, 2016

@pvnr0082t
I have modified the package.json and removed the minfied files.
Please help me review it again, thanks.

@x09326 x09326 assigned pvnr0082t and unassigned x09326 May 13, 2016
],
"authors": [
"Peter Müller",
"Andreas Lind"
Copy link
Contributor

Choose a reason for hiding this comment

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

@x09326
Copy link
Contributor Author

x09326 commented May 13, 2016

@pvnr0082t
I added the email of the authors.
Please help me review it, thanks.

@pvnr0082t
Copy link
Contributor

@PeterDaveHello I think this PR is ok. Please review it again, thank you.

"/home/munter/git/one-color/lib/plugins/opaquer.js",
"/home/munter/git/one-color/lib/plugins/rotate.js",
"/home/munter/git/one-color/lib/plugins/saturate.js",
"/home/munter/git/one-color/lib/plugins/toAlpha.js"
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks strange here, maybe we should drop this, @x09326 can you check this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How to check it?
Ask the author?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, you can confirm with its author, in fact, this could be your chance to know about js and map file.

Copy link

Choose a reason for hiding this comment

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

We noticed that one in the last production build. Caused by one of the browserify plugins that can't handle relative path source maps if I recall correctly. Ping @papandreou

Choose a reason for hiding this comment

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

We did fix that with One-com/one-color@14762b7 but for some reason we didn't release a new version after that :/

Choose a reason for hiding this comment

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

Fixed in 3.0.2.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @Munter @papandreou 👍

@PeterDaveHello
Copy link
Contributor

@x09326 you can update the the latest version, or just drop the map file in v3.0.1

@x09326 x09326 changed the title Add onecolor v3.0.1 with npm auto-update config Add onecolor v3.0.2 with npm auto-update config May 16, 2016
@x09326
Copy link
Contributor Author

x09326 commented May 16, 2016

@pvnr0082t
I have updated the latest version v3.0.2.
Please help me review it, thanks.

@PeterDaveHello
Copy link
Contributor

hmmmm ... maybe we don't need to wait for the fix next time, I shouldn't make this too complex.

@PeterDaveHello
Copy link
Contributor

anyway, thanks you all.

"name": "onecolor",
"filename": "one-color-all.js",
"description": "Javascript color object with implicit color space conversions. Supports RGB, HSV, HSL and CMYK with alpha channel.",
"license": "BSD-2-clause",
Copy link
Contributor

Choose a reason for hiding this comment

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

Since I didn't see this from upstream, @x09326 let's send a PR to contribute upstream having this.

@PeterDaveHello
Copy link
Contributor

@x09326 you didn't rebase your branch with the latest master branch.

@x09326
Copy link
Contributor Author

x09326 commented May 16, 2016

Okay, I will send a PR to contribute upstream.

@pvnr0082t
I have rebased it.
Please help me review it, thanks.

@pvnr0082t
Copy link
Contributor

@PeterDaveHello I think this PR is ok. Please review it again, thank you.

@pvnr0082t pvnr0082t assigned PeterDaveHello and unassigned x09326 May 16, 2016
@PeterDaveHello PeterDaveHello merged commit d73b8c7 into cdnjs:master May 16, 2016
@x09326 x09326 deleted the onecolor branch October 12, 2016 11:56
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

5 participants