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

add dep-check task #2976

Merged
merged 1 commit into from Apr 25, 2016
Merged

add dep-check task #2976

merged 1 commit into from Apr 25, 2016

Conversation

erwinmombay
Copy link
Member

No description provided.

@erwinmombay
Copy link
Member Author

@cramforce we ok with something like this?

screen shot 2016-04-22 at 6 16 51 pm

@erwinmombay erwinmombay force-pushed the graph-check branch 2 times, most recently from 3db6552 to 8abc737 Compare April 23, 2016 01:29
@erwinmombay
Copy link
Member Author

might expand it to be a deeper graph for better error reporting and just add find helpers

cramforce
cramforce reviewed Apr 23, 2016
View reviewed changes
build-system/tasks/jsify-css.js
@@ -0,0 +1,66 @@
/**
Copy link
Member

Choose a reason for hiding this comment

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

Wanna make this a separate PR? Or put it into the description.

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 can split it into a separate PR. thought i would need it but browserify didnt blow up for some reason w/o the CSS dependency being there. will investigate

@erwinmombay erwinmombay force-pushed the graph-check branch 3 times, most recently from e837e0b to 6417655 Compare April 25, 2016 07:36
@erwinmombay erwinmombay changed the title [WIP] add dep-check task add dep-check task Apr 25, 2016
@erwinmombay erwinmombay force-pushed the graph-check branch 10 times, most recently from 7e6b84a to 5b068dc Compare April 25, 2016 08:31
@erwinmombay
Copy link
Member Author

error on travis:

screen shot 2016-04-25 at 1 31 27 am

@erwinmombay
Copy link
Member Author

added #2988 for improvement to consolidate the browserify run with build task.

@erwinmombay erwinmombay force-pushed the graph-check branch 4 times, most recently from aa411b0 to efd1213 Compare April 25, 2016 08:48
@erwinmombay erwinmombay force-pushed the graph-check branch 2 times, most recently from 6eca819 to 2b58930 Compare April 25, 2016 13:50
cramforce
cramforce reviewed Apr 25, 2016
View reviewed changes
build-system/dep-check-config.js
*/
var RuleConfigDef;

exports.rules = [
Copy link
Member

Choose a reason for hiding this comment

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

<3

Copy link
Member Author

Choose a reason for hiding this comment

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

@cramforce will need to whitelist analytics to get green build. is it intentional that we are importing the services directly there? doesnt seem right. https://github.com/ampproject/amphtml/blob/master/extensions/amp-analytics/0.1/amp-analytics.js#L22

Copy link
Member

Choose a reason for hiding this comment

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

It is intentional! The analytics package provides that service.

Basically we want a forbidden deps test from everywhere else to that cid file

@cramforce
Copy link
Member

Super awesome!

LGTM

@erwinmombay erwinmombay merged commit a13d8ec into ampproject:master Apr 25, 2016
@erwinmombay erwinmombay mentioned this pull request May 23, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@cramforce cramforce

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@erwinmombay @cramforce