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.

Sign up for GitHub

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

Jump to bottom

A few NPM changes #3609

Closed
wants to merge 2 commits into from
Closed

A few NPM changes #3609

wants to merge 2 commits into from

Conversation

jeffbcross
Copy link
Contributor

I still want to do some manual testing and output comparisons locally, so please don't auto-submit. I'll add a comment once I've finished those.

@jeffbcross jeffbcross added this to the alpha-35 milestone Aug 12, 2015
@jeffbcross jeffbcross added comp: build/pipeline action: review The PR is still awaiting reviews from at least one requested reviewer labels Aug 12, 2015
@jeffbcross
Copy link
Contributor Author

I need to fix one thing in the publish script, which is copying over "index.js" from http.

@jeffbcross
Copy link
Contributor Author

I fixed issues with copying over index*, package.json, and pubspec.yaml into the angular2 distribution. Still need to test installing and using http on a sample project.

@jeffbcross
Copy link
Contributor Author

I'm going to remove 406a4c4 from this PR and not block the release on that one, since it will require more work to make module paths play well with residing in a different context. I.e. the System.register calls for http/http should be angular2/http, etc. I'll still publish the http module as its own package, ngHttp, and its bundle will still be included in the angular2 distribution.

@jeffbcross
Copy link
Contributor Author

Alright, PR reduced to bundles and typings.

jeffbcross
jeffbcross reviewed Aug 12, 2015
View reviewed changes
scripts/publish/npm_publish.sh
@@ -6,11 +6,12 @@ ROOT_DIR=$(cd $(dirname $0)/../..; pwd)
cd $ROOT_DIR

gulp clean
gulp build.js.prod build.js.dev build.js.cjs benchpress.bundle
gulp build.js.prod build.js.dev build.js.cjs benchpress.bundle bundles.js docs/typings
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could probably remove the first three builds here since they are also called by bundles.js.

@jeffbcross jeffbcross assigned jeffbcross and unassigned mprobst Aug 12, 2015
@jeffbcross
Copy link
Contributor Author

Reviewed with @alexeagle and he gave verbal LGTM after I make changes we discussed.

@jeffbcross jeffbcross added pr_state: LGTM action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Aug 12, 2015
@jeffbcross
feat(npm): publish bundles and their typings in npm distribution
fc3a882 
The same bundles that are published to https://code.angularjs.org
are now included in a "bundles" folder inside the npm distribution.

Closes angular#3555
@jeffbcross
feat(npm): add typescript block to package.json
9e1d521 
This makes it simple to run the `tsd link` command in a project
to automatically include paths to typings files. The definitions
also include transitive dependencies of rx.d.ts and es6-promise.d.ts.

Closes angular#3590
@jeffbcross jeffbcross added the action: merge The PR is ready for merge by the caretaker label Aug 12, 2015
@mary-poppins
Copy link

User @jeffbcross does not have PR merging privlidges.

@mary-poppins mary-poppins removed the action: merge The PR is ready for merge by the caretaker label Aug 12, 2015
mprobst
mprobst reviewed Aug 12, 2015
View reviewed changes
scripts/publish/npm_publish.sh

NPM_DIR=$ROOT_DIR/dist/npm
rm -fr $NPM_DIR
FILES='!(test|e2e_test|docs)'
DTS_FILES='*.d.ts'
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: call this DTS_FILES_GLOB? Or just inline it? It's rather unlikely to change, ever.

@mprobst
Copy link
Contributor

mprobst commented Aug 12, 2015

LGTM!

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees

@jeffbcross jeffbcross

Labels
action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews cla: yes
Projects
None yet
Milestone
alpha-35
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@jeffbcross @mary-poppins @mprobst @googlebot