Skip to content

build: restructure release output #3695

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
Closed
wants to merge 4 commits into from
Closed

build: restructure release output #3695

wants to merge 4 commits into from

Conversation

devversion
Copy link
Member

@devversion devversion commented Mar 21, 2017 •
edited
Loading

Work in Progress PR (for review)

Based on top of #3608

Note: AOT Travis check will likely fail at the moment. Not updated yet.

cc. @jelbourn

@devversion devversion added the in progress This issue is currently in progress label Mar 21, 2017
@devversion devversion self-assigned this Mar 21, 2017
@googlebot googlebot added the cla: no PR author must sign Google's Contributor License Agreement: https://opensource.google.com/docs/cla label Mar 21, 2017
@devversion devversion force-pushed the build/package branch 4 times, most recently from 809c2f5 to ebf4aa9 Compare March 22, 2017 19:48
@googlebot
Copy link

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

@devversion devversion force-pushed the build/package branch 2 times, most recently from 809c2f5 to f8ec65d Compare March 22, 2017 19:57
crisbeto added 3 commits March 22, 2017 20:58
@crisbeto @devversion
feat: angular 4 compatibility
7b3367a 
* Bumps the required Angular version to 4 and TypeScript to 2.1.6.
* Fixes deprecation warnings for `<template>` usages.
* Fixes any unit and e2e test failures.
* Includes the new animations module and switches any components that use animations to it.
* Fixes issues with the various test apps.

Fixes angular#3357.
Fixes angular#3336.
FIxes angular#3301.
@crisbeto @devversion
fix: select animation not being reset on close
a93cb46
@crisbeto @devversion
chore: remove TODO, it works as expected.
00dc42d
@jelbourn jelbourn added cla: yes PR author has agreed to Google's Contributor License Agreement and removed cla: no PR author must sign Google's Contributor License Agreement: https://opensource.google.com/docs/cla labels Mar 22, 2017
jelbourn
jelbourn reviewed Mar 22, 2017
View reviewed changes
tools/gulp/constants.ts Outdated
@@ -17,6 +29,10 @@ export const HTML_MINIFIER_OPTIONS = {
removeAttributeQuotes: false
};

export const UGLIFYJS_OPTIONS = {
preserveComments: 'license'
};
Copy link
Member

Choose a reason for hiding this comment

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

I think we should just define these where we call uglify if we're only doing it in one place.

tools/gulp/tasks/aot.ts Outdated
// As a workaround for https://github.com/angular/angular/issues/12249, we need to
// copy the Material ESM output inside of the demo-app output.
task('aot:copy-release', () => {
copySync(DIST_RELEASE, join(DIST_DEMOAPP, 'material'));
Copy link
Member

Choose a reason for hiding this comment

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

Any reason not to do

task('aot:copy-release', () => src(DIST_RELEASE).pipe(dest(join(DIST_DEMOAPP, 'material'))));

?
(I thought this was the most "gulp-y" way to do it, but I could be wrong)

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried, but it really felt weird because I would need to specify a glob to match all files. (Just specifying the directory didn't work)

task('aot:copy-release', () => src(DIST_RELEASE + '**/*').pipe(dest(join(DIST_DEMOAPP, 'material'))));

It is definitely a gulp-y way, but I think it's just not very clear and also we would need to have a glob.

tools/gulp/tasks/release.ts Outdated
/** Task that combines intermediate build artifacts into the release package structure. */
task(':package:release', [
':package:fix-metadata',
':package:metadata',
Copy link
Member

Choose a reason for hiding this comment

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

Should only call :packagae:metadata, which then depends on :package:fix-metadata?
(to make sure the order of operations is correct since this isn't a runSequence)

Copy link
Member Author

Choose a reason for hiding this comment

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

👍

@googlebot googlebot added cla: no PR author must sign Google's Contributor License Agreement: https://opensource.google.com/docs/cla and removed cla: yes PR author has agreed to Google's Contributor License Agreement labels Mar 22, 2017
@devversion
Copy link
Member Author

Closing as we are merging this together with @jelbourn's changes: #3751

@devversion devversion closed this Mar 24, 2017
@devversion devversion deleted the build/package branch November 11, 2017 10:19
@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 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jelbourn jelbourn jelbourn left review comments

Assignees

@devversion devversion

Labels
cla: no PR author must sign Google's Contributor License Agreement: https://opensource.google.com/docs/cla in progress This issue is currently in progress
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@devversion @googlebot @jelbourn @crisbeto