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.

Sign up for GitHub

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 validation for readmes #7088

Merged
merged 22 commits into from
Jul 24, 2020
Merged

Add validation for readmes #7088

Changes from 1 commit
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
fe97679
Add utility functions
sarah-witt Jul 8, 2020
a6021c4
Add readme validation
sarah-witt Jul 8, 2020
fe2ecff
Move function
sarah-witt Jul 8, 2020
3318868
Add to CI
sarah-witt Jul 8, 2020
efa6bb3
Go back to original comment
sarah-witt Jul 8, 2020
f22b00f
Simplify logic
sarah-witt Jul 8, 2020
a376e88
Check for overview and setup
sarah-witt Jul 8, 2020
3106143
Merge branch 'master' of https://github.com/DataDog/integrations-core…
sarah-witt Jul 9, 2020
c5b3493
Fix style
sarah-witt Jul 9, 2020
d19e5fe
Simplify error
sarah-witt Jul 9, 2020
0f36d80
Test for exact wording
sarah-witt Jul 9, 2020
a2ad088
Fix style
sarah-witt Jul 9, 2020
9a2104b
Check that path exists
sarah-witt Jul 13, 2020
4c16bea
Update extensions
sarah-witt Jul 13, 2020
890a81c
Remove test for non tile ints
sarah-witt Jul 13, 2020
774b10c
Update datadog_checks_dev/datadog_checks/dev/tooling/commands/validat…
sarah-witt Jul 22, 2020
cd0dd3c
Break if readme is invalid
sarah-witt Jul 23, 2020
4fa05a7
Merge branch 'sarah/readme-validation' of https://github.com/DataDog/…
sarah-witt Jul 23, 2020
954d346
Fix style
sarah-witt Jul 24, 2020
7deaf2c
Fix style
sarah-witt Jul 24, 2020
127556e
Update to have display queue
sarah-witt Jul 24, 2020
d5f7142
Add noqa
sarah-witt Jul 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Check that path exists
  • Loading branch information
@sarah-witt
sarah-witt committed Jul 13, 2020
commit 9a2104be69bd515d8cd46c15f0852f90d0157693
16 changes: 12 additions & 4 deletions datadog_checks_dev/datadog_checks/dev/tooling/commands/validate/readmes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@
# All rights reserved
# Licensed under a 3-clause BSD style license (see LICENSE)
import re
from os import path

import click

from ...utils import complete_valid_checks, get_valid_integrations, read_readme_file
from ...utils import complete_valid_checks, get_root, get_valid_integrations, read_readme_file
from ..console import CONTEXT_SETTINGS, abort, echo_failure, echo_info, echo_success

IMAGE_EXTENSIONS = {"png", "jpg"}
Expand Down Expand Up @@ -49,17 +50,24 @@ def readmes(ctx, integration):
if ext in line:
AlexandreYang marked this conversation as resolved.
Show resolved Hide resolved
IMAGE_REGEX = (
rf".*https:\/\/raw\.githubusercontent\.com\/DataDog\/"
rf"{re.escape(repo)}\/master\/{re.escape(integration)}\/images\/.*.\.{ext}.*"
rf"{re.escape(repo)}\/master\/({re.escape(integration)}\/images\/.*.\.{ext}).*"
)

if not re.match(IMAGE_REGEX, line):
match = re.match(IMAGE_REGEX, line)
if not match:
errors = True
echo_failure(f"{integration} readme file does not have a valid image file on line {line_no}")
Copy link
Collaborator

@nmuesch nmuesch Jul 24, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

One thing I noticed is that the other validations print the integration name once when there is an error, then print each error message on a newline with a slight indent, to group errors per integration
Example in the manifest validations - https://github.com/DataDog/integrations-core/blob/master/datadog_checks_dev/datadog_checks/dev/tooling/commands/validate/manifest.py#L289-L301

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like that format! I updated the PR

echo_info(
f"This image path must be in the form: "
f"https://raw.githubusercontent.com/DataDog/{repo}/master/{integration}/images/<IMAGE_NAME>"
)
continue

rel_path = match.groups()[0]
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think need to abort above or put this in an else, otherwise we never see the failure message above if match doesn't exist.

Copy link
Collaborator

Choose a reason for hiding this comment

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

EDIT: sorry you do see the error message above, but it gets drowned out a bit by a stacktrace

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, I updated to break if there's a failure!

if rel_path:
file_path = path.join(get_root(), rel_path)
if not path.exists(file_path):
errors = True
echo_failure(f"{integration} image: {rel_path} is linked in its readme but does not exist")

if not (has_overview and has_setup) and integration not in NON_TILE_INTEGRATIONS:
errors = True
Expand Down