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 Lando development environment #23213

Merged
merged 15 commits into from Aug 20, 2019

Conversation

@westonruter
Copy link
Member

commented Jul 9, 2019

In order to facilitate contributing to AMP, this PR seeks to automate much of the manual steps outlined in the Getting Started End-to-end guide and to provide a containerized development environment via Lando, which is a cross-platform user-friendly wrapper around Docker.

With Lando installed, the steps to get started with contributing to AMP would be simplified to git clone and lando start. The gulp serve command would then be run automatically so that you should be able to right away navigate to https://amphtml.lndo.site/ as opposed to http://localhost:8000/.

๐Ÿ‘‰ Lando would be an optional path for developers to get up and running quickly for contributing in a containerized environment, as long as their machine meets the system requirements for Lando. Doing local dev directly on the host machine would continue to be an option.

Setup

Install the latest release of Lando (which includes Docker): https://github.com/lando/lando/releases (see System Requirements).

Add to your machine's hosts file:

127.0.0.1 amphtml.lndo.site

You'll trust the default Lando CA on your machine so that you can go to https://amphtml.lando.site without any SSL warnings.

lando start

Output from running lando start for the first time or lando rebuild should be:

$ lando rebuild -y
Rising anew like a fire phoenix from the ashes! Rebuilding app...
Killing amphtml_appserver_1 ... done
Going to remove amphtml_appserver_1
Removing amphtml_appserver_1 ... done
Pulling appserver ... done
appserver uses an image, skipping
landoproxyhyperion5000gandalfedition_proxy_1 is up-to-date
Creating amphtml_appserver_1 ... done
Killing amphtml_appserver_1 ... done
Starting amphtml_appserver_1 ... done
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  2314  100  2314    0     0   1384      0  0:00:01  0:00:01 --:--:--  1383
Installing Yarn!
> Downloading tarball...

[1/2]: https://yarnpkg.com/latest.tar.gz --> /tmp/yarn.tar.gz.rrg43w6OGZ
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100    93  100    93    0     0    666      0 --:--:-- --:--:-- --:--:--   669
100   609    0   609    0     0    323      0 --:--:--  0:00:01 --:--:--   493
100 1145k  100 1145k    0     0   256k      0  0:00:04  0:00:04 --:--:-- 1067k

[2/2]: https://yarnpkg.com/latest.tar.gz.asc --> /tmp/yarn.tar.gz.rrg43w6OGZ.asc
100    97  100    97    0     0   1311      0 --:--:-- --:--:-- --:--:--  1311
100   613    0   613    0     0   2348      0 --:--:-- --:--:-- --:--:--  598k
100   832  100   832    0     0   2245      0 --:--:-- --:--:-- --:--:--  2245
> Verifying integrity...
gpg: Signature made Mon Apr 29 18:16:55 2019 UTC
gpg:                using RSA key 6D98490C6F1ACDDD448E45954F77679369475BAA
gpg: Good signature from "Yarn Packaging <yarn@dan.cx>" [unknown]
gpg: WARNING: This key is not certified with a trusted signature!
gpg:          There is no indication that the signature belongs to the owner.
Primary key fingerprint: 72EC F46A 56B4 AD39 C907  BBB7 1646 B01B 86E5 0310
     Subkey fingerprint: 6D98 490C 6F1A CDDD 448E  4595 4F77 6793 6947 5BAA
> GPG signature looks good
> Extracting to ~/.yarn...
> Adding to $PATH...
> We've added the following to your /root/.bashrc
> If this isn't the profile of your current shell then please add the following to your correct profile:

export PATH="$HOME/.yarn/bin:$HOME/.config/yarn/global/node_modules/.bin:$PATH"

> Successfully installed Yarn 1.16.0! Please open another terminal where the `yarn` command will now be available.
Killing amphtml_appserver_1 ... done
Starting amphtml_appserver_1 ... done
yarn global v1.16.0
[1/4] Resolving packages...
[2/4] Fetching packages...
[3/4] Linking dependencies...
[4/4] Building fresh packages...
success Installed "gulp-cli@2.2.0" with binaries:
      - gulp
Done in 6.71s.
Killing amphtml_appserver_1 ... done
Starting amphtml_appserver_1 ... done
yarn install v1.16.0
$ node build-system/check-package-manager.js
Detected node version v10.16.0 (latest LTS).
Detected gulp version 4.0.2.
Detected yarn version 1.16.0 (stable). Installing packages...
[1/5] Validating package.json...
[2/5] Resolving packages...
success Already up-to-date.
Done in 5.43s.
Killing amphtml_appserver_1 ... done
Starting amphtml_appserver_1 ... done
Waiting until appserver service is ready...

BOOMSHAKALAKA!!!

Your app has started up correctly.
Here are some vitals:

 NAME            amphtml
 LOCATION        /Users/westonruter/repos/amphtml
 SERVICES        appserver
 APPSERVER URLS  http://localhost:32999
                 http://amphtml.lndo.site
                 https://amphtml.lndo.site

lando watch

Then running lando watch fires up gulp watch in the container:

$ lando watch
yarn run v1.16.0
$ /app/node_modules/.bin/gulp watch
[18:03:44] Using gulpfile /app/gulpfile.js
[18:03:44] Starting 'watch'...
[18:03:45] All packages in node_modules are up to date.
[18:03:45] Running watch. Press Ctrl + C to cancel...
[18:03:45] Building version 1907091759130 of the runtime for local testing with the prod AMP config.
[18:03:45] โคท Use --config={canary|prod} with your gulp watch command to specify which config to apply.
[18:03:45] Building all AMP extensions.
[18:03:45] โคท Use --noextensions to skip building extensions.
[18:03:45] โคท Use --extensions=amp-foo,amp-bar to choose which extensions to build.
[18:03:45] โคท Use --extensions=minimal_set to build just the extensions needed to load article.amp.html.
[18:03:45] โคท Use --extensions_from=examples/foo.amp.html to build extensions from example docs.
[18:03:53] Recompiled all CSS files into build/ (7.442 s)
[18:03:54] Processed 3p/nameframe.max.html (1.272 s)
[18:03:54] Processed 3p/recaptcha.max.html (1.272 s)
[18:03:54] Processed 3p/frame.max.html (1.284 s)
[18:04:08] Compiled examiner.max.js (10.807 s)
[18:05:47] Compiled amp-google-vrview-image-0.1.max.js โ†’ amp-google-vrview-image-latest.max.js (18.151 s)
[18:05:49] Compiled amp-kaltura-player-0.1.max.js โ†’ amp-kaltura-player-latest.max.js (17.366 s)
[18:05:49] Compiled amp-connatix-player-0.1.max.js โ†’ amp-connatix-player-latest.max.js (17.000 s)
[18:05:49] Compiled amp-google-document-embed-0.1.max.js โ†’ amp-google-document-embed-latest.max.js (16.870 s)
[18:05:49] Compiled amp-iframe-0.1.max.js โ†’ amp-iframe-latest.max.js (16.445 s)
[18:05:49] Compiled amp-install-serviceworker-0.1.max.js โ†’ amp-install-serviceworker-latest.max.js (16.095 s)
[18:05:49] Compiled amp-jwplayer-0.1.max.js โ†’ amp-jwplayer-latest.max.js (15.866 s)
[18:05:49] Compiled amp-vk-0.1.max.js โ†’ amp-vk-latest.max.js (15.309 s)
[18:05:49] Compiled amp-izlesene-0.1.max.js โ†’ amp-izlesene-latest.max.js (15.532 s)
[18:05:49] Compiled amp-accordion-0.1.max.js โ†’ amp-accordion-latest.max.js (14.910 s)
[18:05:49] Compiled amp-byside-content-0.1.max.js โ†’ amp-byside-content-latest.max.js (14.930 s)
[18:05:49] Compiled amp-sidebar-0.1.max.js โ†’ amp-sidebar-latest.max.js (14.253 s)
[18:05:49] Compiled amp-playbuzz-0.1.max.js โ†’ amp-playbuzz-latest.max.js (13.283 s)
[18:05:49] Compiled amp-viz-vega-0.1.max.js โ†’ amp-viz-vega-latest.max.js (13.061 s)
[18:06:01] Compiled amp-viewer-integration-0.1.max.js โ†’ amp-viewer-integration-latest.max.js (19.442 s)
[18:06:02] Compiled integration.js (18.367 s)
[18:06:07] Compiled amp-audio-0.1.max.js โ†’ amp-audio-latest.max.js (18.775 s)
[18:06:07] Compiled amp-apester-media-0.1.max.js โ†’ amp-apester-media-latest.max.js (18.414 s)
[18:06:08] Compiled amp-ad-exit-0.1.max.js โ†’ amp-ad-exit-latest.max.js (18.789 s)
[18:06:08] Compiled amp-geo-0.1.max.js โ†’ amp-geo-latest.max.js (18.496 s)
[18:06:09] Compiled amp-mustache-0.1.max.js (18.099 s)
[18:06:09] Compiled amp-form-0.1.max.js โ†’ amp-form-latest.max.js (17.662 s)
[18:06:09] Compiled amp-truncate-text-0.1.max.js โ†’ amp-truncate-text-latest.max.js (17.444 s)
[18:06:09] Compiled amp-anim-0.1.max.js โ†’ amp-anim-latest.max.js (17.601 s)
[18:06:09] Compiled alp.max.js (17.459 s)
[18:06:09] Compiled amp-date-countdown-0.1.max.js โ†’ amp-date-countdown-latest.max.js (16.927 s)
[18:06:09] Compiled amp-slides-0.1.max.js โ†’ amp-slides-latest.max.js (15.589 s)
[18:06:09] Compiled amp-imgur-0.1.max.js โ†’ amp-imgur-latest.max.js (16.234 s)
[18:06:09] Compiled amp-date-display-0.1.max.js โ†’ amp-date-display-latest.max.js (16.535 s)
[18:06:09] Compiled amp-o2-player-0.1.max.js โ†’ amp-o2-player-latest.max.js (16.023 s)
[18:06:10] Compiled amp-timeago-0.1.max.js โ†’ amp-timeago-latest.max.js (15.593 s)
[18:06:10] Compiled amp-soundcloud-0.1.max.js โ†’ amp-soundcloud-latest.max.js (15.950 s)
[18:06:10] Compiled amp-springboard-player-0.1.max.js โ†’ amp-springboard-player-latest.max.js (15.382 s)
[18:06:10] Compiled amp-riddle-quiz-0.1.max.js โ†’ amp-riddle-quiz-latest.max.js (15.874 s)
[18:06:10] Compiled amp-vine-0.1.max.js โ†’ amp-vine-latest.max.js (15.383 s)
[18:06:10] Compiled amp-hulu-0.1.max.js โ†’ amp-hulu-latest.max.js (14.913 s)
[18:06:10] Compiled amp-autocomplete-0.1.max.js โ†’ amp-autocomplete-latest.max.js (15.203 s)
[18:06:10] Compiled amp-instagram-0.1.max.js โ†’ amp-instagram-latest.max.js (14.812 s)
[18:06:10] Compiled amp-selector-0.1.max.js โ†’ amp-selector-latest.max.js (14.291 s)
[18:06:10] Compiled amp-image-lightbox-0.1.max.js โ†’ amp-image-lightbox-latest.max.js (14.743 s)
[18:06:10] Compiled amp-app-banner-0.1.max.js โ†’ amp-app-banner-latest.max.js (13.721 s)
[18:06:10] Compiled amp-image-viewer-0.1.max.js โ†’ amp-image-viewer-latest.max.js (14.110 s)
[18:06:11] Compiled amp-sticky-ad-1.0.max.js โ†’ amp-sticky-ad-latest.max.js (14.658 s)
[18:06:13] Compiled amp-user-location-0.1.max.js โ†’ amp-user-location-latest.max.js (15.900 s)
[18:06:13] Compiled amp-orientation-observer-0.1.max.js โ†’ amp-orientation-observer-latest.max.js (16.035 s)
[18:06:13] Compiled amp-position-observer-0.1.max.js โ†’ amp-position-observer-latest.max.js (15.881 s)
[18:06:13] Compiled amp-viewer-assistance-0.1.max.js โ†’ amp-viewer-assistance-latest.max.js (15.517 s)
[18:06:13] Compiled amp-consent-0.1.max.js โ†’ amp-consent-latest.max.js (15.198 s)
[18:06:14] Compiled amp-fx-flying-carpet-0.1.max.js โ†’ amp-fx-flying-carpet-latest.max.js (14.916 s)
[18:06:14] Compiled amp-video-docking-0.1.max.js โ†’ amp-video-docking-latest.max.js (15.583 s)
[18:06:15] Compiled amp-image-slider-0.1.max.js โ†’ amp-image-slider-latest.max.js (15.059 s)
[18:06:15] Compiled amp-pan-zoom-0.1.max.js โ†’ amp-pan-zoom-latest.max.js (14.700 s)
[18:06:15] Compiled amp-social-share-0.1.max.js โ†’ amp-social-share-latest.max.js (14.397 s)
[18:06:16] Compiled amp-experiment-0.1.max.js โ†’ amp-experiment-latest.max.js (15.143 s)
[18:06:16] Compiled recaptcha.js (15.221 s)
[18:06:21] Compiled ampcontext-lib.js (18.203 s)
[18:06:21] Compiled amp-auto-ads-0.1.max.js โ†’ amp-auto-ads-latest.max.js (18.703 s)
[18:06:21] Compiled polyfills.js (17.887 s)
[18:06:21] Compiled amp-font-0.1.max.js โ†’ amp-font-latest.max.js (17.599 s)
[18:06:22] Compiled amp-mustache-0.2.max.js โ†’ amp-mustache-latest.max.js (17.261 s)
[18:06:23] Compiled amp-access-laterpay-0.2.max.js โ†’ amp-access-laterpay-latest.max.js (18.079 s)
[18:06:23] Compiled amp-access-laterpay-0.1.max.js (17.563 s)
[18:06:30] Compiled amp-pinterest-0.1.max.js โ†’ amp-pinterest-latest.max.js (24.248 s)
[18:06:30] Compiled iframe-transport-client-lib.js (23.993 s)
[18:06:32] Compiled amp-web-push-0.1.max.js โ†’ amp-web-push-latest.max.js (24.849 s)
[18:06:33] Compiled amp-bodymovin-animation-0.1.max.js โ†’ amp-bodymovin-animation-latest.max.js (23.423 s)
[18:06:33] Compiled amp-access-scroll-0.1.max.js โ†’ amp-access-scroll-latest.max.js (22.922 s)
[18:06:33] Compiled amp-facebook-page-0.1.max.js โ†’ amp-facebook-page-latest.max.js (22.328 s)
[18:06:33] Compiled amp-fx-collection-0.1.max.js โ†’ amp-fx-collection-latest.max.js (22.002 s)
[18:06:33] Compiled amp-gist-0.1.max.js โ†’ amp-gist-latest.max.js (21.336 s)
[18:06:33] Compiled amp-facebook-0.1.max.js โ†’ amp-facebook-latest.max.js (21.757 s)
[18:06:33] Compiled amp-twitter-0.1.max.js โ†’ amp-twitter-latest.max.js (20.920 s)
[18:06:33] Compiled amp-reddit-0.1.max.js โ†’ amp-reddit-latest.max.js (20.472 s)
[18:06:33] Compiled amp-yotpo-0.1.max.js โ†’ amp-yotpo-latest.max.js (19.882 s)
[18:06:33] Compiled ww.max.js (19.040 s)
[18:06:33] Compiled amp-facebook-comments-0.1.max.js โ†’ amp-facebook-comments-latest.max.js (17.929 s)
[18:06:33] Compiled amp-auto-lightbox-0.1.max.js โ†’ amp-auto-lightbox-latest.max.js (18.318 s)
[18:06:33] Compiled amp-facebook-like-0.1.max.js โ†’ amp-facebook-like-latest.max.js (17.654 s)
[18:06:33] Compiled amp-mathml-0.1.max.js โ†’ amp-mathml-latest.max.js (17.060 s)
[18:06:33] Compiled amp-gwd-animation-0.1.max.js โ†’ amp-gwd-animation-latest.max.js (17.406 s)
[18:06:33] Compiled amp-3d-gltf-0.1.max.js โ†’ amp-3d-gltf-latest.max.js (16.754 s)
[18:06:33] Compiled amp-crypto-polyfill-0.1.max.js โ†’ amp-crypto-polyfill-latest.max.js (15.211 s)
[18:06:33] Compiled amp-lightbox-0.1.max.js โ†’ amp-lightbox-latest.max.js (16.148 s)
[18:06:33] Compiled amp-beopinion-0.1.max.js โ†’ amp-beopinion-latest.max.js (15.581 s)
[18:06:34] Compiled amp-reach-player-0.1.max.js โ†’ amp-reach-player-latest.max.js (14.987 s)
[18:06:34] Compiled amp-inputmask-0.1.max.js โ†’ amp-inputmask-latest.max.js (15.204 s)
[18:06:34] Compiled amp-experiment-1.0.max.js (14.882 s)
[18:06:34] Compiled amp-fit-text-0.1.max.js โ†’ amp-fit-text-latest.max.js (14.685 s)
[18:06:34] Compiled amp-base-carousel-0.1.max.js โ†’ amp-base-carousel-latest.max.js (14.630 s)
[18:06:34] Compiled amp-call-tracking-0.1.max.js โ†’ amp-call-tracking-latest.max.js (14.244 s)
[18:06:34] Compiled amp-share-tracking-0.1.max.js โ†’ amp-share-tracking-latest.max.js (14.022 s)
[18:06:34] Compiled amp-link-rewriter-0.1.max.js โ†’ amp-link-rewriter-latest.max.js (13.872 s)
[18:06:34] Compiled amp-story-auto-ads-0.1.max.js โ†’ amp-story-auto-ads-latest.max.js (13.549 s)
[18:06:34] Compiled amp-user-notification-0.1.max.js โ†’ amp-user-notification-latest.max.js (13.010 s)
[18:06:35] Compiled amp-access-poool-0.1.max.js โ†’ amp-access-poool-latest.max.js (13.635 s)
[18:06:35] Compiled amp-action-macro-0.1.max.js โ†’ amp-action-macro-latest.max.js (13.390 s)
[18:06:36] Compiled amp-brid-player-0.1.max.js โ†’ amp-brid-player-latest.max.js (12.737 s)
[18:06:36] Compiled amp-subscriptions-google-0.1.max.js โ†’ amp-subscriptions-google-latest.max.js (13.746 s)
[18:06:36] Compiled amp-brightcove-0.1.max.js โ†’ amp-brightcove-latest.max.js (11.204 s)
[18:06:36] Compiled amp-dailymotion-0.1.max.js โ†’ amp-dailymotion-latest.max.js (11.768 s)
[18:06:36] Compiled amp-bind-0.1.max.js โ†’ amp-bind-latest.max.js (12.506 s)
[18:06:36] Compiled amp-ooyala-player-0.1.max.js โ†’ amp-ooyala-player-latest.max.js (10.401 s)
[18:06:36] Compiled amp-ima-video-0.1.max.js โ†’ amp-ima-video-latest.max.js (10.908 s)
[18:06:36] Compiled amp-wistia-player-0.1.max.js โ†’ amp-wistia-player-latest.max.js (10.110 s)
[18:06:36] Compiled amp-nexxtv-player-0.1.max.js โ†’ amp-nexxtv-player-latest.max.js (9.769 s)
[18:06:36] Compiled amp-vimeo-0.1.max.js โ†’ amp-vimeo-latest.max.js (9.377 s)
[18:06:36] Compiled amp-powr-player-0.1.max.js โ†’ amp-powr-player-latest.max.js (7.911 s)
[18:06:36] Compiled amp-video-iframe-0.1.max.js โ†’ amp-video-iframe-latest.max.js (8.478 s)
[18:06:36] Compiled amp-video-0.1.max.js โ†’ amp-video-latest.max.js (9.088 s)
[18:06:36] Compiled amp-mowplayer-0.1.max.js โ†’ amp-mowplayer-latest.max.js (6.783 s)
[18:06:36] Compiled amp-youtube-0.1.max.js โ†’ amp-youtube-latest.max.js (7.679 s)
[18:06:37] Compiled amp-viqeo-player-0.1.max.js โ†’ amp-viqeo-player-latest.max.js (7.304 s)
[18:06:37] Compiled amp-delight-player-0.1.max.js โ†’ amp-delight-player-latest.max.js (6.560 s)
[18:06:39] Compiled amp-carousel-0.1.max.js โ†’ amp-carousel-latest.max.js (8.554 s)
[18:06:39] Compiled amp-addthis-0.1.max.js โ†’ amp-addthis-latest.max.js (8.234 s)
[18:06:39] Compiled amp-gfycat-0.1.max.js โ†’ amp-gfycat-latest.max.js (7.902 s)
[18:06:40] Compiled amp-viewer-host.max.js (8.265 s)
[18:06:41] Compiled amp-3q-player-0.1.max.js โ†’ amp-3q-player-latest.max.js (9.074 s)
[18:06:48] Compiled amp-ad-network-adsense-impl-0.1.max.js โ†’ amp-ad-network-adsense-impl-latest.max.js (14.129 s)
[18:06:49] Compiled amp-dynamic-css-classes-0.1.max.js โ†’ amp-dynamic-css-classes-latest.max.js (13.214 s)
[18:06:49] Compiled amp-ad-custom-0.1.max.js โ†’ amp-ad-custom-latest.max.js (13.674 s)
[18:06:50] Compiled amp-ad-network-gmossp-impl-0.1.max.js โ†’ amp-ad-network-gmossp-impl-latest.max.js (11.202 s)
[18:06:50] Compiled amp-ad-network-cloudflare-impl-0.1.max.js โ†’ amp-ad-network-cloudflare-impl-latest.max.js (12.421 s)
[18:06:51] Compiled amp-embedly-card-0.1.max.js โ†’ amp-embedly-card-latest.max.js (10.997 s)
[18:06:51] Compiled amp-recaptcha-input-0.1.max.js โ†’ amp-recaptcha-input-latest.max.js (11.461 s)
[18:06:51] Compiled amp-ad-network-adzerk-impl-0.1.max.js โ†’ amp-ad-network-adzerk-impl-latest.max.js (10.641 s)
[18:06:51] Compiled amp-mraid-0.1.max.js โ†’ amp-mraid-latest.max.js (8.828 s)
[18:06:51] Compiled amp-ad-network-triplelift-impl-0.1.max.js โ†’ amp-ad-network-triplelift-impl-latest.max.js (9.609 s)
[18:06:51] Compiled amp-live-list-0.1.max.js โ†’ amp-live-list-latest.max.js (8.520 s)
[18:06:51] Compiled amp-ad-network-fake-impl-0.1.max.js โ†’ amp-ad-network-fake-impl-latest.max.js (8.076 s)
[18:06:51] Compiled amp-ad-0.1.max.js โ†’ amp-ad-latest.max.js (7.190 s)
[18:06:52] Compiled amp-animation-0.1.max.js โ†’ amp-animation-latest.max.js (4.650 s)
[18:06:52] Compiled amp-ad-network-doubleclick-impl-0.1.max.js โ†’ amp-ad-network-doubleclick-impl-latest.max.js (6.170 s)
[18:06:52] Compiled amp-list-0.1.max.js โ†’ amp-list-latest.max.js (4.247 s)
[18:06:52] Compiled amp-smartlinks-0.1.max.js โ†’ amp-smartlinks-latest.max.js (3.844 s)
[18:06:53] Compiled amp-skimlinks-0.1.max.js โ†’ amp-skimlinks-latest.max.js (3.551 s)
[18:06:53] Compiled amp-analytics-0.1.max.js โ†’ amp-analytics-latest.max.js (3.273 s)
[18:06:54] Compiled amp-access-0.1.max.js โ†’ amp-access-latest.max.js (2.296 s)
[18:06:58] Compiled amp-next-page-0.1.max.js โ†’ amp-next-page-latest.max.js (3.766 s)
[18:07:00] Compiled amp-subscriptions-0.1.max.js โ†’ amp-subscriptions-latest.max.js (2.348 s)
[18:07:00] Applied AMP config (prod, localDev) to alp.max.js
[18:07:01] Compiled amp-script-0.1.max.js โ†’ amp-script-latest.max.js (1.782 s)
[18:07:03] Compiled amp-lightbox-gallery-0.1.max.js โ†’ amp-lightbox-gallery-latest.max.js (2.595 s)
[18:07:06] Compiled amp-story-0.1.max.js (1.515 s)
[18:07:09] Compiled amp-story-1.0.max.js โ†’ amp-story-latest.max.js (1.695 s)
[18:07:09] Applied AMP config (prod, localDev) to integration.js
[18:07:15] Compiled amp-date-picker-0.1.max.js โ†’ amp-date-picker-latest.max.js (4.049 s)
[18:07:19] Compiled amp.js (2.046 s)
[18:07:19] Applied AMP config (prod, localDev) to amp.js
[18:07:27] Compiled amp-esm.js (1.811 s)
[18:07:27] Applied AMP config (prod, localDev) to amp-esm.js
[18:07:27] Finished 'watch' after 3.7 min

Todo

  • Update E2E setup instructions to provide Lando as option.

@googlebot googlebot added the cla: yes label Jul 9, 2019

@westonruter westonruter force-pushed the westonruter:add/lando branch 2 times, most recently from 229d97f to c65c57f Jul 9, 2019

@westonruter westonruter marked this pull request as ready for review Jul 10, 2019

@westonruter

This comment has been minimized.

Copy link
Member Author

commented Jul 10, 2019

@rsimha Thoughts on this?

@westonruter

This comment has been minimized.

Copy link
Member Author

commented Jul 10, 2019

In regards to the Local AMP extension, it seems to work fine but there are few errors in the console:

image

image

I'm not sure if these are normal.

@mrjoro mrjoro requested a review from rsimha Aug 8, 2019

@mrjoro

This comment has been minimized.

Copy link
Member

commented Aug 8, 2019

@rsimha
Copy link
Collaborator

left a comment

Hi @westonruter, sorry about the delay in reviewing this. I was traveling when I first saw it, and made a mental note to try it out locally before reviewing.

I've added a few comments and looped in a few reviewers since I don't fully understand the scope of all the changes you've made. If not all these changes are critical to your workflow, maybe you could break this up into separate PRs that can be individually tested (and reverted if necessary)?

cmd: "yarn gulp"
watch:
service: appserver
cmd: "yarn gulp watch"

This comment has been minimized.

Copy link
@rsimha

rsimha Aug 16, 2019

Collaborator

The sum total of all our gulp commands used during development can be found by running gulp help. Other commands that might be useful to add here: check-types, dist, unit, integration, and e2e.

build-system/app.js Outdated Show resolved Hide resolved
build-system/app.js Outdated Show resolved Hide resolved
build-system/check-package-manager.js Outdated Show resolved Hide resolved
build-system/shadow-viewer.js Outdated Show resolved Hide resolved
testing/local-amp-chrome-extension/popup.js Outdated Show resolved Hide resolved
@westonruter

This comment has been minimized.

Copy link
Member Author

commented Aug 16, 2019

@rsimha Thanks!

If not all these changes are critical to your workflow, maybe you could break this up into separate PRs that can be individually tested (and reverted if necessary)?

Yes, I'm happy to split it up into separate PRs. The changes to the Chrome extension are independent to the Lando environment, though the Lando environment depends on it to customize the URL.

Similarly, the changes to shadow-viewer.js are independent of Lando, though any dev who wants to use a host other than localhost will need the change (as in the case of the Lando env).

@wassgha wassgha self-requested a review Aug 16, 2019

@wassgha

This comment has been minimized.

Copy link
Collaborator

commented Aug 16, 2019

Chrome Extension changes LGTM although I agree they should probably be a separate PR

@rsimha rsimha requested a review from estherkim Aug 16, 2019

@estherkim
Copy link
Contributor

left a comment

I like the initiative to simplify the set up process for new contributors. However I have a few concerns -

If it's ok, I'd like to clarify the use cases for adding Lando support before moving forward.

@westonruter

This comment has been minimized.

Copy link
Member Author

commented Aug 16, 2019

I like the initiative to simplify the set up process for new contributors. However I have a few concerns -

@estherkim Thanks for reviewing. Local dev should still be a valid path, but using Lando can be an alternative for developers who prefer a containerized environment that's also easy to use. IMO using something Docker-based should be preferred since it is more secure.

westonruter added 8 commits Jul 9, 2019

@westonruter westonruter force-pushed the westonruter:add/lando branch from 7bd9dd2 to 1c947e9 Aug 18, 2019

@westonruter

This comment has been minimized.

Copy link
Member Author

commented Aug 18, 2019

(Rebased latest commits from master.)

@westonruter

This comment has been minimized.

Copy link
Member Author

commented Aug 18, 2019

Chrome Extension changes LGTM although I agree they should probably be a separate PR

@wassgha I cherry-picked commits into separate PR for the Local AMP extension: #24029. Once merged I'll rebase this branch to omit them from this PR.

@rsimha

This comment has been minimized.

Copy link
Collaborator

commented Aug 19, 2019

IMO using something Docker-based should be preferred since it is more secure.

I agree with @estherkim's point that due to the hardware requirements, this isn't easy for every developer to do. In general, I don't think a docker-based environment should be a part of the default recommended developer workflow for AMP, since not everyone can use it, and it's likely to increase the maintenance burden on our side. Having said that, we're definitely supportive of smaller changes to the infrastructure so that those who'd like to use environments like Lando can do so.

Re: the changes in this PR, perhaps you could do the following?

  • Split off all changes to the local AMP extension, local serving, etc into separate self-contained PRs
  • Update the warning text in check-package-manager.js to include a note on npx gulp in a separate PR
  • The last question is whether .lando.yml should be checked in or not. This can be proposed / discussed via its own PR (or we can reuse this one!)

@estherkim @westonruter WDYT?

@estherkim

This comment has been minimized.

Copy link
Contributor

commented Aug 19, 2019

  • Split off all changes to the local AMP extension, local serving, etc into separate self-contained PRs
  • Update the warning text check-package-manager.js to include a note on npx gulp in a separate PR
  • The last question is whether .lando.yml should be checked in or not. This can be proposed / discussed via its own PR

This all sounds great to me. Thanks @rsimha for providing a clear way forward.

@westonruter

This comment has been minimized.

Copy link
Member Author

commented Aug 19, 2019

@rsimha Sounds good.

  • Split off all changes to the local AMP extension, local serving, etc into separate self-contained PRs

For Local AMP extension, see pull request here: #24029

  • Update the warning text in check-package-manager.js to include a note on npx gulp in a separate PR

See outstanding question in #23213 (comment)

  • The last question is whether .lando.yml should be checked in or not. This can be proposed / discussed via its own PR (or we can reuse this one!)

๐Ÿ‘

@westonruter
Copy link
Member Author

left a comment

I've cherry-picked the commits related to non-localhost gulp serve improvements: #24066.

build-system/shadow-viewer.js Outdated Show resolved Hide resolved
build-system/app.js Outdated Show resolved Hide resolved
build-system/app.js Outdated Show resolved Hide resolved
westonruter added 2 commits Aug 20, 2019
Revert "Prevent warning 'Found gulp in an unexpected location' when rโ€ฆ
โ€ฆepo in non-amphtml dir"

This reverts commit 57b9857.
Merge branch 'master' of github.com:ampproject/amphtml into add/lando
* 'master' of github.com:ampproject/amphtml: (32 commits)
  โœจ Make tweet id a bindable attribute (#23953)
  ๐Ÿ— Update Local AMP extension to allow custom base URLs (#24029)
  ๐Ÿ— Improve serving from non-localhost host (#24066)
  Preventing half pixels. (#24050)
  Update callout-vendors.js (#23218)
  ๐Ÿ— Fixes to `check-package-manager.js` (#24060)
  Rename AMP_MODE to __AMP_MODE. (#24052)
  Story media performance metrics. (#23962)
  Updating Story amp-sidebar width documentation. (#23894)
  Fixes race condition in amp-video-iframe (#24033)
  Rename ampExtendedElements to __AMP_EXTENDED_ELEMENTS (#24056)
  ๐Ÿ—๐Ÿšฎ Enable property inlining (#24053)
  โœจamp-ads: Added optional params for Directadvert network (#23724)
  <amp-experiment> style mutation fix and improvment (#23669)
  ๐Ÿ› Allow http protocol for noscript > img fallbacks for parity with amp-img (#21686)
  ๐Ÿ— Refactor transform-log-asserts (#24028)
  Automatically preconnect to source origins on page loads. (#24045)
  Support visibility API in the ampdoc (#23799)
  Amphtml visual tests should use relative path against root (#24042)
  FIX: check all fields' dirtiness on AMP form init (#23978)
  ...
@westonruter

This comment has been minimized.

Copy link
Member Author

commented Aug 20, 2019

Excellent. I merged master and now the only changes left in this PR are Lando-specific (the .lando.yml).

@rsimha
rsimha approved these changes Aug 20, 2019
Copy link
Collaborator

left a comment

@westonruter Thanks for your patience with this PR! I've added a couple of comments that may make the config more maintainable.

About merging .lando.yml:

  • I'm fine with adding it to the repo since it doesn't affect the default developer workflow, and there is precedent for adding dotfiles.
  • However, I'm hesitant to actively recommend docker-based development in our dev docs because most AMP developers work on current or previous gen Macbooks that do not meet the hardware requirements. (I asked @jridgewell about this yesterday.)
.lando.yml Outdated Show resolved Hide resolved
.lando.yml Show resolved Hide resolved
westonruter added 4 commits Aug 20, 2019
@rsimha
rsimha approved these changes Aug 20, 2019
Copy link
Collaborator

left a comment

LGTM!

@estherkim
Copy link
Contributor

left a comment

LGTM, thank you!

@rsimha rsimha added the WG: infra label Aug 20, 2019

@rsimha rsimha merged commit 416d52c into ampproject:master Aug 20, 2019

12 of 16 checks passed

LGTM analysis: JavaScript No code changes detected
Details
ampproject/owners-check ampproject/owners-check
Details
ampproject/pr-deploy Ready to create a test site.
Details
ampproject/tests/unit (local-changes) Tests were not required
Details
ampproject/bundle-size ฮ” +0.00KB | no approval necessary
Details
ampproject/tests/e2e (local) 353 tests passed
Details
ampproject/tests/integration (local) 272 tests passed
Details
ampproject/tests/integration (saucelabs) 419 tests passed
Details
ampproject/tests/integration (single-pass) 214 tests passed
Details
ampproject/tests/unit (local) 10201 tests passed
Details
ampproject/tests/unit (saucelabs) 7917 tests passed
Details
cla/google All necessary CLAs are signed
codecov/patch Coverage not affected when comparing 5b44147...f40f07e
Details
codecov/project 79.96% (+0.06%) compared to 5b44147
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
percy/amphtml Visual review automatically approved, no visual changes found.
Details

@westonruter westonruter deleted the westonruter:add/lando branch Aug 20, 2019

thekorn pushed a commit to edelight/amphtml that referenced this pull request Sep 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You canโ€™t perform that action at this time.