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 incremental-dom v0.2.0 #5823

Merged
merged 1 commit into from
Jan 17, 2016
Merged

Conversation

maruilian11
Copy link
Contributor

git repo url: https://github.com/google/incremental-dom
Watch 72
Star 1,483
Fork 46

@LeaYeh could u help me check this PR for #5729 ? thank you!

  • Not found on cdnjs repo
  • No already exist issue and PR
  • Notable popularity
  • Project has public repository on famous online hosting platform (or been hosted on npm)
  • Has valid tags for each versions (for git auto-update)
  • Auto-update setup
  • Auto-update target/source is valid.
  • Auto-update filemap is correct

{
"basePath": "dist",
"files": [
"incremental-dom*.+(js|map)"
Copy link
Contributor

Choose a reason for hiding this comment

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

why not **/*?

@LeaYeh
Copy link
Contributor

LeaYeh commented Oct 13, 2015

@maruilian11 why you add npm autoupdate config but not git config?

@maruilian11
Copy link
Contributor Author

@LeaYeh i'm not sure if i use git auto-update, i can get dist folder.
cc #5729

@PeterDaveHello
Copy link
Contributor

npm is good

@LeaYeh
Copy link
Contributor

LeaYeh commented Oct 16, 2015

@maruilian11 ok, i think this pr is ok, plz rebase it again :)
@PeterDaveHello plz check this pr

@PeterDaveHello
Copy link
Contributor

@LeaYeh You should ping me after the rebase, not now ...

@PeterDaveHello
Copy link
Contributor

@LeaYeh And the pr's title is too long.

@LeaYeh
Copy link
Contributor

LeaYeh commented Oct 16, 2015

@PeterDaveHello sorry for that, and what is the max len of title?

@PeterDaveHello
Copy link
Contributor

I think, maybe within 50 chars, like add incremental-dom v0.2.0

@maruilian11 maruilian11 changed the title add incremental-dom v0.2.0 with npm auto-update, close #5729 add incremental-dom v0.2.0 Oct 16, 2015
@maruilian11 maruilian11 force-pushed the incremental-dom branch 2 times, most recently from 8289766 to ff7b731 Compare October 20, 2015 05:24
@maruilian11
Copy link
Contributor Author

@LeaYeh is that ok?

@LeaYeh
Copy link
Contributor

LeaYeh commented Oct 22, 2015

i think it is fine
@PeterDaveHello

@PeterDaveHello
Copy link
Contributor

should rebase ... @LeaYeh don't you know that or forgot again?

@maruilian11
Copy link
Contributor Author

@LeaYeh i have rebased it ~

@LeaYeh
Copy link
Contributor

LeaYeh commented Nov 7, 2015

ok, this pr is ok.
@PeterDaveHello could you check this pr for me?

@PeterDaveHello
Copy link
Contributor

Don't you need to add the manually minified file incremental-dom-cjs.min.js into requiredFiles array?
And also I wonder if we really need it, maybe it's not for browser, if it's for browser, the official developer may minify it as like incremental-dom-min.js, please confirm that @LeaYeh .

@maruilian11
Copy link
Contributor Author

@LeaYeh could you help me check the PR again?
i use !(incremental-dom-cjs*) to not include incremental-dom-cjs* in dist

@maruilian11 maruilian11 removed their assignment Dec 2, 2015
@maruilian11
Copy link
Contributor Author

@Amomo could you help me check this PR? thank you~

@maruilian11
Copy link
Contributor Author

ping @LeaYeh

@PeterDaveHello
Copy link
Contributor

i use !(incremental-dom-cjs_) to not include incremental-dom-cjs_ in dist

Did you confirm that is incremental-dom-cjs for browser or not?

I wonder if this project if for browser usage or not ...

@maruilian11
Copy link
Contributor Author

PeterDaveHello added a commit that referenced this pull request Jan 17, 2016
@PeterDaveHello PeterDaveHello merged commit 7aa6939 into cdnjs:master Jan 17, 2016
@maruilian11 maruilian11 deleted the incremental-dom branch January 17, 2016 14:30
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