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
new optimizer readme #346
new optimizer readme #346
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, a few small comments.
|
||
Currently the following options are supported: | ||
```shell | ||
$ npx @ampproject/toolbox-cli myFile.html |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no command required?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so, but I'll try once it's deployed
packages/optimizer/README.md
Outdated
|
||
// retrieve the latest runtime version | ||
const runtimeVersion = require('amp-toolbox-runtime-version'); | ||
const ampOptimiser = require('@ampproject/toolbox-optimizer'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this should be ampOptimizer
, for consistency with the package name. (As an Australian used to British spelling, I am very reluctant to mention this…)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lol - fixed
|
||
[![npm version](https://badge.fury.io/js/amp-toolbox-optimizer.svg)](https://badge.fury.io/js/amp-toolbox-optimizer) | ||
|
||
AMP Optimizer - the performance optimizations of the AMP Cache on your own site. | ||
AMP Optimizer is a tool to server-side enhance the rendering performance of AMP pages. AMP Optimizer implements [AMP performance best practices](https://amp.dev/documentation/guides-and-tutorials/optimize-and-measure/optimize_amp?format=websites) and supports [AMP server-side-rendering](https://amp.dev/documentation/guides-and-tutorials/optimize-and-measure/server-side-rendering?format=websites). By default, it will perform the following optimizations: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's worth mentioning that the optimizer generates valid AMP (given valid input) somewhere here for clarity (even if briefly) especially since this is a change from previous versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point! Done!
packages/optimizer/README.md
Outdated
* The AMP boilerplate gets removed. Instead the `v0.css` is added. | ||
* AMP layout classes are added to the `amp-img` extension. | ||
* `<link rel="amphtml" href="canonical.amp.html">` gets added linking to the valid AMP version. | ||
This transformer supports the following option: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/option/options/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
packages/optimizer/README.md
Outdated
AMP Runtime being loaded, which improves load times. | ||
Example: | ||
``` | ||
const ampOptimiser = require('@ampproject/toolbox-optimizer'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe change this to "optimizer" too…
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
to: | ||
``` | ||
// globally | ||
const optimizer = AmpOptimzer.create({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/AmpOptimzer/AmpOptimizer/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Michael!
packages/optimizer/README.md
Outdated
|
||
// retrieve the latest runtime version | ||
const runtimeVersion = require('amp-toolbox-runtime-version'); | ||
const ampOptimiser = require('@ampproject/toolbox-optimizer'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lol - fixed
packages/optimizer/README.md
Outdated
* The AMP boilerplate gets removed. Instead the `v0.css` is added. | ||
* AMP layout classes are added to the `amp-img` extension. | ||
* `<link rel="amphtml" href="canonical.amp.html">` gets added linking to the valid AMP version. | ||
This transformer supports the following option: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
packages/optimizer/README.md
Outdated
AMP Runtime being loaded, which improves load times. | ||
Example: | ||
``` | ||
const ampOptimiser = require('@ampproject/toolbox-optimizer'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
|
||
Currently the following options are supported: | ||
```shell | ||
$ npx @ampproject/toolbox-cli myFile.html |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so, but I'll try once it's deployed
|
||
[![npm version](https://badge.fury.io/js/amp-toolbox-optimizer.svg)](https://badge.fury.io/js/amp-toolbox-optimizer) | ||
|
||
AMP Optimizer - the performance optimizations of the AMP Cache on your own site. | ||
AMP Optimizer is a tool to server-side enhance the rendering performance of AMP pages. AMP Optimizer implements [AMP performance best practices](https://amp.dev/documentation/guides-and-tutorials/optimize-and-measure/optimize_amp?format=websites) and supports [AMP server-side-rendering](https://amp.dev/documentation/guides-and-tutorials/optimize-and-measure/server-side-rendering?format=websites). By default, it will perform the following optimizations: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point! Done!
No description provided.