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

Remove dependencies on yarn #11279

Merged
merged 11 commits into from
Sep 13, 2017
1 change: 0 additions & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ matrix:
jwt:
secure: "Wze0F0vGL0UcxryOx1n/vcuD5LIMGyR+69Nc6IWLoRvZBbbIpFwVFhDE6rE9ranIXiA2Hc684N4sV8ASfNDF8RRSB+jyLov159qwgji2rBxIfQ/4kuDV2vYoAJvYMz8m42kwx5FV2VV9awqMMt8mwU3wYIrKIaVCxB34uV86KIlDlbrHxt17Bm5EIiUmwi9r1AAnW/63vVRUN264D77oB4j9UQ759PfD6BDwEt54O87KurNIaLseNCr1IvzfL8veEsZ3uTbLC1GtgHfR4IGgkS2YyN2QIk06VZWeRDEOalS3RcY0nDkbCmBywxIGObnrpEMzOpjBiOb2fxLoLvvpjlla5W84zJGfWE6q4T9IvkyHuDJE+sft5B+arjMIeA6PIeUhKdV27+6qqDEf7fILZ/U/Ekn9ds4zSV8hekAZPUyyPncOeyWppCIJ8sOeCrsebkRjH1BoX/d+FE+nP0bN/XkBpIi/nManx5FyS/kqjQWGKmvsFQfEWlSUaZi7XtEQEjvBizRkzvpJanSDaoiTDS2Keulmwii3XRId51FuGtnfDZFeggLaMTKGfBX9DlPkccwYAZe6vPNfYk1pNgEj6AtnifEhYVEO+aAuWhEnJ86od+1wDOL/h+a2XY6h8/gFBywsD95p7sXPfdVDCKgwagiBo+Hw5MNjztVF7lszg1A="
cache:
yarn: true
directories:
- node_modules
- .gem
Expand Down
3 changes: 1 addition & 2 deletions build-system/SERVING.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,14 @@ limitations under the License.
- node + npm
- gulp (installed globally)
- java 8
- yarn (see https://yarnpkg.com/)

## Steps
```bash
git clone https://github.com/ampproject/amphtml.git
cd amphtml
# Checkout a tag
git checkout 123456789
yarn
npm install
gulp clean
# We only need to build the css files, no need to generate `max` files
gulp build --css-only
Expand Down
12 changes: 6 additions & 6 deletions build-system/pr-check.js
Original file line number Diff line number Diff line change
Expand Up @@ -380,17 +380,17 @@ function main(argv) {
process.exit(1);
}

// Make sure changes to package.json also update yarn.lock.
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of removing this check, maybe repurpose it to make sure that package.json and package-lock.json are updated in lock step? (Heh!)

Copy link
Member Author

Choose a reason for hiding this comment

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

Fair enough, PTAL

if (files.indexOf('package.json') != -1 && files.indexOf('yarn.lock') == -1) {
// Make sure changes to package.json also update package-lock.json.
if (files.indexOf('package.json') != -1 && files.indexOf('package-lock.json') == -1) {
console.error(fileLogPrefix, util.colors.red('ERROR:'),
'Updates to', util.colors.cyan('package.json'),
'must be accompanied by a corresponding update to',
util.colors.cyan('yarn.lock'));
util.colors.cyan('package-lock.json'));
console.error(fileLogPrefix, util.colors.yellow('NOTE:'),
'To update', util.colors.cyan('yarn.lock'), 'after changing',
'To update', util.colors.cyan('package-lock.json'), 'after changing',
util.colors.cyan('package.json') + ',', 'run',
'"' + util.colors.cyan('yarn install') + '"',
'and include the change to', util.colors.cyan('yarn.lock'),
'"' + util.colors.cyan('npm purge && npm install') + '"',
'and include the change to', util.colors.cyan('package-lock.json'),
'in your PR.');
process.exit(1);
}
Expand Down
2 changes: 1 addition & 1 deletion contributing/DEVELOPING.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ Before you start developing in AMP, check out these resources:

For most developers the instructions in the [Getting Started Quick Start Guide](getting-started-quick.md) will be sufficient for building/running/testing during development. This section provides a more detailed reference.

The Quick Start Guide's [One-time setup](getting-started-quick.md#one-time-setup) has instructions for installing Node.js, Yarn, and Gulp which you'll need before running these commands.
The Quick Start Guide's [One-time setup](getting-started-quick.md#one-time-setup) has instructions for installing Node.js, npm, and Gulp which you'll need before running these commands.

| Command | Description |
| ----------------------------------------------------------------------- | --------------------------------------------------------------------- |
Expand Down
15 changes: 9 additions & 6 deletions contributing/getting-started-e2e.md
Original file line number Diff line number Diff line change
Expand Up @@ -151,16 +151,17 @@ Now run `git remote -v` again and notice that you have set up your upstream alia

Now that you have all of the files copied locally you can actually build the code and run a local server to try things out.

amphtml uses Node.js, the yarn package manager and the Gulp build system to build amphtml and start up a local server that lets you try out your changes. Installing these and getting amphtml built is straightforward:
amphtml uses Node.js, the npm package manager and the Gulp build system to build amphtml and start up a local server that lets you try out your changes. Installing these and getting amphtml built is straightforward:

* Install [NodeJS](https://nodejs.org/) (which includes npm).
* Install [NodeJS](https://nodejs.org/) version >= 4.7 (which includes npm)

* Install [yarn](https://yarnpkg.com/en/docs/install)
* Install [npm](https://www.npmjs.com/get-npm) version >= 5

Choose a reason for hiding this comment

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

Nit: Since installing node also installs npm, this should probably be "upgrade npm to version >= 5".

Copy link
Member Author

Choose a reason for hiding this comment

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

Done, PTAL


* In your local repository directory (e.g. `~/src/ampproject/amphtml`), install the packages that AMP uses by running
```
yarn
npm install
```

You should see a progress indicator and some messages scrolling by. You may see some warnings about optional dependencies that are generally safe to ignore.

* For some local testing we refer to fake local URLs in order to simulate referencing third party URLs. This requires extra setup so your browser will know that these URLs actually point to your local server.
Expand All @@ -171,12 +172,14 @@ amphtml uses Node.js, the yarn package manager and the Gulp build system to buil

* The AMP Project uses Gulp as our build system. Gulp uses a configuration file ([gulpfile.js](https://github.com/ampproject/amphtml/blob/master/gulpfile.js)) to build amphtml (including the amphtml javascript) and to start up the Node.js server with the proper settings. You don't really have to understand exactly what it is doing at this point--you just have to install it and use it.

You can install Gulp using yarn:
You can install Gulp using npm:

```
yarn global add gulp
npm install -g gulp
```

On some platform the preceding command might require elevated privileges using `sudo`.

Now whenever you're ready to build amphtml and start up your local server, simply go to your local repository directory and run:

```
Expand Down
9 changes: 5 additions & 4 deletions contributing/getting-started-quick.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@ This Quick Start guide is the TL;DR version of the longer [end-to-end guide](get

* [Install and set up Git](https://help.github.com/articles/set-up-git/); in the "Authenticating" step of that page use SSH instead of HTTPS

* Install [NodeJS](https://nodejs.org/)
* Install [NodeJS](https://nodejs.org/) version >= 4.7

* Install [yarn](https://yarnpkg.com/en/docs/install)
* Install [npm](https://www.npmjs.com/get-npm)` version >= 5

* Install Gulp by running `yarn global add gulp`
* Install Gulp by running `npm install -g gulp` (on some platforms this command might require elevated privileges using `sudo`)

* Add this line to your hosts file (`/etc/hosts` on Mac or Linux, `%SystemRoot%\System32\drivers\etc\hosts` on Windows):

Expand All @@ -48,7 +48,7 @@ This Quick Start guide is the TL;DR version of the longer [end-to-end guide](get
* Go to the branch: `git checkout <branch name>`

# Build AMP & run a local server
* Make sure you have the latest packages (after you pull): `yarn`
* Make sure you have the latest packages (after you pull): `npm install`
Copy link
Contributor

Choose a reason for hiding this comment

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

Another useful command is to run npm prune to make sure unnecessary dependencies are removed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added it below in the "Create commits to contain your changes" section

* Start the server: `gulp`
* Access your server at [http://localhost:8000](http://localhost:8000)
* Access your sample pages at [http://localhost:8000/examples](http://localhost:8000/examples)
Expand All @@ -62,6 +62,7 @@ This Quick Start guide is the TL;DR version of the longer [end-to-end guide](get
# Create commits to contain your changes

* Edit files in your favorite editor
* If you edited `package.json` run `npm prune && npm install` to generate an updated `package-lock.json` file
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Add a comma after "If you edited package.json"

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

* Add each file you change: `git add <file>`
* Create a commit: `git commit -m "<your commit message>"`
* Instead of `add`ing each file individually you can use the `-a` flag on the commit instead
Expand Down
Loading