Added og:image and alt text checks to linter for Stories #755

Dbrown910 · 2020-05-07T17:44:08Z

Initial updates to linter as apart of Linter UI project

googlebot · 2020-05-07T17:44:11Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

googlebot · 2020-05-07T19:11:58Z

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

ℹ️ Googlers: Go here for more info.

sebastianbenz

Thanks! Looks good. Left a few comments (mostly nits). Can you please add tests as well.

packages/linter/src/rules/ImagesHaveAltText.ts

packages/linter/src/rules/MetadataIncludesOGImageSrc.ts

googlebot · 2020-05-11T20:10:20Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

sebastianbenz

Looks good! Few nits.

sebastianbenz · 2020-05-11T22:01:16Z

packages/linter/src/rules/MetadataIncludesOGImageSrc.ts

      ? this.warn(`Missing og:image property or content source`)
      : this.pass();
  }
  meta() {
    return {
-      url:
-        "https://html.spec.whatwg.org/multipage/parsing.html#determining-the-character-encoding",
+      url: "https://ogp.me/",


Would it besser to link to a stories specific documentation page?

I can swap it for the SEO for Stories post

sebastianbenz · 2020-05-11T22:01:41Z

packages/linter/tests/local.ts

@@ -102,5 +104,31 @@ assertWarn(
  runLocalTest(BookendExists, "local/BookendExists-3/source.html")
 );

+assertPass(
+  `${MetadataIncludesOGImageSrc.name} - <meta property="og:image"> is presentt`,


typo: s/presentt/present

sebastianbenz · 2020-05-11T22:02:36Z

packages/linter/tests/local/ImagesHaveAltText-1/source.html

+        <meta property="og:image:height" content="1920">
+        <meta name="twitter:card" content="summary_large_image">
+        <link rel="canonical" href="https://onl.st/linter-tester">
+        <style amp-custom>


Can you clean up this file and remove everything that is not relevant for the test (e.g. CSS)

sebastianbenz · 2020-05-11T22:02:57Z

packages/linter/tests/local/ImagesHaveAltText-2/source.html

+        <meta name="twitter:card" content="summary_large_image">
+        <link rel="canonical" href="https://onl.st/linter-tester">
+        <style amp-custom>
+            .pbvnsupp amp-img.pbvnsup>img {


Same as above, cleanup. the file

googlebot · 2020-05-12T16:59:42Z

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

ℹ️ Googlers: Go here for more info.

sebastianbenz · 2020-05-12T17:00:08Z

I've manually verified the CLA (@Dbrown910 is a Googler)

sebastianbenz

Great - thanks!

Added og:image and alt text checks to linter for Stories

fa67f68

googlebot added the cla: no label May 7, 2020

Dbrown910 closed this May 7, 2020

Dbrown910 reopened this May 7, 2020

sebastianbenz added cla: yes and removed cla: no labels May 7, 2020

sebastianbenz requested changes May 7, 2020

View reviewed changes

packages/linter/src/rules/ImagesHaveAltText.ts Outdated Show resolved Hide resolved

packages/linter/src/rules/MetadataIncludesOGImageSrc.ts Outdated Show resolved Hide resolved

packages/linter/src/rules/MetadataIncludesOGImageSrc.ts Outdated Show resolved Hide resolved

Damani Brown added 2 commits May 8, 2020 17:07

Merge branch 'master' of https://github.com/ampproject/amp-toolbox

66eb2ea

Minor fixes to linter checks. Added testing for new checks

e8a19b7

googlebot added cla: no and removed cla: yes labels May 11, 2020

Dbrown910 added 2 commits May 11, 2020 16:20

Removed unused var

e9f7ce1

Recommitting for CI build, No code changes

b41933a

sebastianbenz reviewed May 11, 2020

View reviewed changes

Code cleanup

c3e266d

sebastianbenz added cla: yes and removed cla: no labels May 12, 2020

sebastianbenz approved these changes May 12, 2020

View reviewed changes

sebastianbenz merged commit b262d7c into ampproject:master May 12, 2020

sebastianbenz added the enhancement label May 13, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added og:image and alt text checks to linter for Stories #755

Added og:image and alt text checks to linter for Stories #755

Dbrown910 commented May 7, 2020 •

edited

Loading

googlebot commented May 7, 2020

googlebot commented May 7, 2020

sebastianbenz left a comment

googlebot commented May 11, 2020

sebastianbenz left a comment

sebastianbenz May 11, 2020

Dbrown910 May 12, 2020

sebastianbenz May 11, 2020

sebastianbenz May 11, 2020

sebastianbenz May 11, 2020

googlebot commented May 12, 2020

sebastianbenz commented May 12, 2020

sebastianbenz left a comment

Added og:image and alt text checks to linter for Stories #755

Added og:image and alt text checks to linter for Stories #755

Conversation

Dbrown910 commented May 7, 2020 • edited Loading

googlebot commented May 7, 2020

What to do if you already signed the CLA

Individual signers

Corporate signers

googlebot commented May 7, 2020

sebastianbenz left a comment

Choose a reason for hiding this comment

googlebot commented May 11, 2020

sebastianbenz left a comment

Choose a reason for hiding this comment

sebastianbenz May 11, 2020

Choose a reason for hiding this comment

Dbrown910 May 12, 2020

Choose a reason for hiding this comment

sebastianbenz May 11, 2020

Choose a reason for hiding this comment

sebastianbenz May 11, 2020

Choose a reason for hiding this comment

sebastianbenz May 11, 2020

Choose a reason for hiding this comment

googlebot commented May 12, 2020

sebastianbenz commented May 12, 2020

sebastianbenz left a comment

Choose a reason for hiding this comment

Dbrown910 commented May 7, 2020 •

edited

Loading