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

Take amp-list routes out of app.js into its own routing file #21214

Merged
merged 4 commits into from
Mar 5, 2019

Conversation

cathyxz
Copy link
Contributor

@cathyxz cathyxz commented Mar 1, 2019

I need to add some new routes for another case for amp-list-load-more but app.js is > 1k loc. This is a pure refactor PR to take the routes relevant to amp-list out of app.js into its own routing file. I updated the relevant routes for amp-list.

@cathyxz cathyxz requested review from torch2424, aghassemi, dreamofabear and erwinmombay and removed request for torch2424 March 1, 2019 23:06
@cathyxz cathyxz changed the title Take amp-list routes out of app.js and give it its own routing file Take amp-list routes out of app.js into its own routing file Mar 1, 2019
@cathyxz cathyxz self-assigned this Mar 2, 2019
@cathyxz
Copy link
Contributor Author

cathyxz commented Mar 2, 2019

Okay I think I have to change some usages of enableCors. Don't know why the presubmit check didn't catch that locally. I'll reassign when this is fixed.

@cathyxz cathyxz removed their assignment Mar 5, 2019
@cathyxz
Copy link
Contributor Author

cathyxz commented Mar 5, 2019

/cc owners @erwinmombay @danielrozenberg @rsimha in case any of you would like to take a look. Otherwise this is merging EOD. ;)

Copy link
Contributor

@rsimha rsimha left a comment

Choose a reason for hiding this comment

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

Owners approval for build-system/

@cathyxz
Copy link
Contributor Author

cathyxz commented Mar 5, 2019

Thank you Raghu!

@cathyxz cathyxz merged commit d8005e5 into ampproject:master Mar 5, 2019
noranazmy pushed a commit to noranazmy/amphtml that referenced this pull request Mar 22, 2019
…ect#21214)

* Break amp-list routes out into own routing file

* Refactor amp-list manual tests and routes out to their own file and folder

* Update other usages of enableCors
bramanudom pushed a commit to bramanudom/amphtml that referenced this pull request Mar 22, 2019
…ect#21214)

* Break amp-list routes out into own routing file

* Refactor amp-list manual tests and routes out to their own file and folder

* Update other usages of enableCors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants