Chore docs crawl #16451

Narretz · 2018-02-12T10:33:05Z

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

What is the current behavior? (You can also link to an open issue here)

What is the new behavior (if this is a feature change)?

Does this PR introduce a breaking change?

Please check if the PR fulfills these requirements

The commit message follows our guidelines
Fix/Feature: Docs have been added/updated
Fix/Feature: Tests have been added; existing tests pass

Other information:

mgol · 2018-02-12T10:36:59Z

scripts/docs.angularjs.org-firebase/functions/.eslintrc.json

+    "no-console": "off",
+
+    // Removed rule "disallow multiple spaces in regular expressions" from recommended eslint rules
+    "no-regex-spaces": "off",


What's the reason for all those exceptions?

mgol · 2018-02-12T10:38:59Z

scripts/docs.angularjs.org-firebase/functions/.eslintrc.json

+  "plugins": [
+    "promise"
+  ],
+  "extends": "eslint:recommended",


Why do we not use our rules but extend the recommended preset?

These are auto-generated by Firebase and I didn't bother to review them, tbh

Can we extend our rules / preset and still allow ES6 stuff? Because we don't allow that for our scripting stuff.

We should allow ES6 for our Node.js stuff in general now that we run the build in Node 8.

Is just the ESLint config generated by Firebase or the whole code in this directory? If the latter then just add "root": true to .eslintrc.json so that it doesn't inherit our config.

gkalpak · 2018-02-12T10:38:40Z

Gruntfile.js

@@ -316,10 +316,10 @@ module.exports = function(grunt) {
        files: [
          // the zip file should not be compressed again.


This commends needs updating now 😃

gkalpak · 2018-02-12T10:41:08Z

Gruntfile.js

@@ -356,7 +356,7 @@ module.exports = function(grunt) {
        options: {
          mode: 'gzip'
        },
-        src: ['**', '!*.zip'],
+        src: ['**', '!**/*.{zip,png,jpeg,jpg}'],


Is the **/ part needed? It doesn't seem so 😕

but aren't the patterns exclusive? So if I do !*.{zip,.. doesn't that only affect files in the first folder?

For some reason, I thought the strings would be concatenated to create a pattern 😥

gkalpak · 2018-02-12T10:47:00Z

Gruntfile.js

-            dest: 'uploadDocs/',
+            dest: 'deploy/docs/',
+            expand: true
+          }


Ooops, extra expand: true} 😁

extra as in "this is not needed"?

You had

expand: true } expand: true }

Fixed on the subsequent commit, though.

gkalpak · 2018-02-12T11:08:48Z

Gruntfile.js

@@ -165,7 +167,8 @@ module.exports = function(grunt) {
      tmp: ['tmp'],
      deploy: [
        'deploy/docs',
-        'deploy/code'
+        'deploy/code',
+        docsScriptFolder + '/functions/html'


Shouldn't this be /functions/content?
(BTW, it might make sense to put some constants that we share accross files into an external file and import that.)

gkalpak · 2018-02-12T11:11:22Z

scripts/docs.angularjs.org-firebase/functions/package-lock.json

@@ -0,0 +1,4265 @@
+{


Does firebase take this into account?

Hm. Firebase started the npm install process when I ran firebase init functions. And npm tells you to commit the lock file, so I did :D

gkalpak · 2018-02-12T11:12:12Z

scripts/docs.angularjs.org-firebase/functions/package.json

+  "name": "functions",
+  "description": "Cloud Functions for Firebase",
+  "scripts": {
+    "lint": "./node_modules/.bin/eslint .",


You don't need the ./node_modules/.bin/ prefix.

this was also auto-generated by firebase

So? 😛
Both npm and yarn will add node_modules/.bin/ to the PATH when executing npm scripts. The current syntax does not run on Windows 😞

It runs on Windows with Git Bash! 👅

gkalpak · 2018-02-12T11:13:59Z

scripts/docs.angularjs.org-firebase/readme.firebase.docs.md

+# Serving locally:
+
+- Run `grunt:prepareDeploy`.
+  This copies docs content files are into deploy/docs and the partials for Search Engine AJAX


files are into --> files into

gkalpak · 2018-02-12T11:14:26Z

scripts/docs.angularjs.org-firebase/functions/package.json

+    "logs": "firebase functions:log"
+  },
+  "dependencies": {
+    "firebase-admin": "~5.8.1",


Do we need this?

Firebase auto-generates this file when you init the functions, so I'd rather not touch it

gkalpak · 2018-02-12T11:17:44Z

scripts/docs.angularjs.org-firebase/functions/index.js

+'use strict';
+
+const functions = require('firebase-functions');
+const path = require('path');


Narretz · 2018-02-12T12:34:33Z

I've now allowed es6 for all our node js scripts

mgol · 2018-02-12T12:45:08Z

.eslintrc-node.json

+  },
+  "parserOptions": {
+    // Required for certain syntax usages
+    "ecmaVersion": 6


Let's change it to 2017 as Node 8 already supports it: https://kangax.github.io/compat-table/es2016plus/

This commit restores serving the plain partials (content) when a docs page is accessed with ?_escaped_fragment_=. The Google Ajax Crawler accesses these urls when the page has `<meta type="fragment" content="!">` is set. During the migration to Firebase, this was lost, which resulted in Google dropping the docs almost completely from the index. We are using a Firebase cloud function to serve the partials. Since we cannot access the static hosted files from the function, we have to deploy them as part of the function directory instead, from which they can be read. Related to angular#16432 Related to angular#16417

Narretz · 2018-02-12T13:35:05Z

@gkalpak @mgol thanks for the reviews!

Follow-up to angular#16451.

Follow-up to #16451. Closes #16452

googlebot added the cla: yes label Feb 12, 2018

mgol suggested changes Feb 12, 2018

View reviewed changes

Narretz force-pushed the chore-docs-crawl branch from b5a07a8 to 1892b18 Compare February 12, 2018 11:13

gkalpak suggested changes Feb 12, 2018

View reviewed changes

Narretz force-pushed the chore-docs-crawl branch from 1892b18 to 538f352 Compare February 12, 2018 12:34

mgol suggested changes Feb 12, 2018

View reviewed changes

Narretz added 4 commits February 12, 2018 13:52

chore(eslint): allow ES6 for node scripts

a9b9b17

chore(code.angularjs.org): don't gzip compressed image files

d496099

chore(deploy): rename deploy folders

55018dd

Narretz force-pushed the chore-docs-crawl branch from 538f352 to ab0675b Compare February 12, 2018 12:52

mgol approved these changes Feb 12, 2018

View reviewed changes

Narretz mentioned this pull request Feb 12, 2018

google.com search results are linking doc pages without stylesheets #16432

Closed

3 tasks

Narretz merged commit 75bf199 into angular:master Feb 12, 2018

Narretz deleted the chore-docs-crawl branch February 12, 2018 13:43

gkalpak added a commit to gkalpak/angular.js that referenced this pull request Feb 12, 2018

chore(docs.angular.js): do not break when deploying

2ee7ce5

Follow-up to angular#16451.

gkalpak mentioned this pull request Feb 12, 2018

chore(docs.angular.js): do not break when deploying #16452

Closed

gkalpak added a commit to gkalpak/angular.js that referenced this pull request Feb 12, 2018

chore(docs.angular.js): do not break when deploying

373e102

Follow-up to angular#16451.

gkalpak added a commit to gkalpak/angular.js that referenced this pull request Feb 12, 2018

chore(docs.angular.js): do not break when deploying

1973fc9

Follow-up to angular#16451.

gkalpak added a commit that referenced this pull request Feb 12, 2018

chore(docs.angular.js): do not break when deploying

fb47918

Follow-up to #16451. Closes #16452

gkalpak added a commit that referenced this pull request Feb 12, 2018

chore(docs.angular.js): do not break when deploying

06f5069

Follow-up to #16451. Closes #16452

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Chore docs crawl #16451

Chore docs crawl #16451

Narretz commented Feb 12, 2018

mgol Feb 12, 2018

mgol Feb 12, 2018

Narretz Feb 12, 2018

mgol Feb 12, 2018

gkalpak Feb 12, 2018

gkalpak Feb 12, 2018

Narretz Feb 12, 2018

gkalpak Feb 12, 2018

gkalpak Feb 12, 2018

Narretz Feb 12, 2018

gkalpak Feb 12, 2018

gkalpak Feb 12, 2018

gkalpak Feb 12, 2018

Narretz Feb 12, 2018

gkalpak Feb 12, 2018

Narretz Feb 12, 2018

gkalpak Feb 12, 2018

Narretz Feb 12, 2018

gkalpak Feb 12, 2018

gkalpak Feb 12, 2018

Narretz Feb 12, 2018

gkalpak Feb 12, 2018

Narretz commented Feb 12, 2018

mgol Feb 12, 2018

Narretz Feb 12, 2018

Narretz commented Feb 12, 2018

		@@ -316,10 +316,10 @@ module.exports = function(grunt) {
		files: [
		// the zip file should not be compressed again.

Chore docs crawl #16451

Chore docs crawl #16451

Conversation

Narretz commented Feb 12, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Narretz commented Feb 12, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Narretz commented Feb 12, 2018