Enhance changelog job with more validation steps #44

abikouo · 2023-02-23T16:58:15Z

This changelog validates that the PR contains a valid changelog file if needed.

gravesm

What's the difference between this PR and the existing changelog workflow? Can we work with the network team to try and incorporate these changes into the other workflow?

.github/workflows/cloud-changelog.yml

abikouo · 2023-03-13T09:24:44Z

What's the difference between this PR and the existing changelog workflow? Can we work with the network team to try and incorporate these changes into the other workflow?

The existing workflow has been updated

gravesm

Could you describe how this is different from the current solution? There are likely people already using the existing action and if this changes the behavior we need to know how it is changing.

Qalthos

I think I'm convinced that this does what is needed more precisely than before, though it is a great deal more complicated now. I think we can merge this, but I would love @cidrblock's eyes on it as well.

cidrblock · 2023-03-22T14:01:19Z

I think this is fine, although in the case of a failure (sys.exit(1)) I'd like to see a bunch of debug level information printed out about what happened during the run. Doc strings, type hints, black and pylint/flake8 erros resolved. Here's some I found with a quick run:

validate_changelog.py:45:38: C0303: Trailing whitespace (trailing-whitespace)
validate_changelog.py:82:0: C0301: Line too long (111/100) (line-too-long)
validate_changelog.py:86:0: C0301: Line too long (104/100) (line-too-long)
validate_changelog.py:149:0: C0304: Final newline missing (missing-final-newline)
validate_changelog.py:15:47: W1401: Anomalous backslash in string: '\.'. String constant might be missing an r prefix. (anomalous-backslash-in-string)
validate_changelog.py:1:0: C0114: Missing module docstring (missing-module-docstring)
validate_changelog.py:14:0: C0116: Missing function or method docstring (missing-function-docstring)
validate_changelog.py:17:0: C0116: Missing function or method docstring (missing-function-docstring)
validate_changelog.py:40:0: C0116: Missing function or method docstring (missing-function-docstring)
validate_changelog.py:47:0: C0116: Missing function or method docstring (missing-function-docstring)
validate_changelog.py:50:7: R1729: Use a generator instead 'any(is_module_or_plugin(x) for x in changes['A'])' (use-a-generator)
validate_changelog.py:55:7: R1729: Use a generator instead 'all(is_documentation_file(x) for x in all_files)' (use-a-generator)
validate_changelog.py:60:0: C0116: Missing function or method docstring (missing-function-docstring)
validate_changelog.py:82:26: C0209: Formatting a regular string which could be a f-string (consider-using-f-string)
validate_changelog.py:86:24: C0209: Formatting a regular string which could be a f-string (consider-using-f-string)
validate_changelog.py:95:14: C0209: Formatting a regular string which could be a f-string (consider-using-f-string)
validate_changelog.py:98:0: C0116: Missing function or method docstring (missing-function-docstring)
validate_changelog.py:104:11: R1732: Consider using 'with' for resource-allocating operations (consider-using-with)
validate_changelog.py:108:0: C0116: Missing function or method docstring (missing-function-docstring)
validate_changelog.py:109:10: C0209: Formatting a regular string which could be a f-string (consider-using-f-string)
validate_changelog.py:110:10: C0209: Formatting a regular string which could be a f-string (consider-using-f-string)
validate_changelog.py:111:4: C0103: Variable name "rc" doesn't conform to snake_case naming style (invalid-name)
validate_changelog.py:117:8: C0103: Variable name "v" doesn't conform to snake_case naming style (invalid-name)
validate_changelog.py:122:0: C0116: Missing function or method docstring (missing-function-docstring)

validate_changelog.py:1:1: D100 Missing docstring in public module
validate_changelog.py:14:1: D103 Missing docstring in public function
validate_changelog.py:15:48: W605 invalid escape sequence '\.'
validate_changelog.py:17:1: D103 Missing docstring in public function
validate_changelog.py:17:1: E302 expected 2 blank lines, found 1
validate_changelog.py:40:1: D103 Missing docstring in public function
validate_changelog.py:45:39: W291 trailing whitespace
validate_changelog.py:47:1: D103 Missing docstring in public function
validate_changelog.py:47:1: E302 expected 2 blank lines, found 1
validate_changelog.py:60:1: D103 Missing docstring in public function
validate_changelog.py:60:1: E302 expected 2 blank lines, found 1
validate_changelog.py:82:101: E501 line too long (111 > 100 characters)
validate_changelog.py:86:101: E501 line too long (104 > 100 characters)
validate_changelog.py:95:84: E231 missing whitespace after ','
validate_changelog.py:98:1: D103 Missing docstring in public function
validate_changelog.py:98:1: E302 expected 2 blank lines, found 1
validate_changelog.py:108:1: D103 Missing docstring in public function
validate_changelog.py:108:1: E302 expected 2 blank lines, found 1
validate_changelog.py:122:1: D103 Missing docstring in public function
validate_changelog.py:122:1: E302 expected 2 blank lines, found 1
validate_changelog.py:130:80: E502 the backslash is redundant between brackets
validate_changelog.py:135:83: E502 the backslash is redundant between brackets
validate_changelog.py:142:1: E305 expected 2 blank lines after class or function definition, found 1
validate_changelog.py:149:39: W292 no newline at end of file
1     D100 Missing docstring in public module
8     D103 Missing docstring in public function
1     E231 missing whitespace after ','
6     E302 expected 2 blank lines, found 1
1     E305 expected 2 blank lines after class or function definition, found 1
2     E501 line too long (111 > 100 characters)
2     E502 the backslash is redundant between brackets
1     W291 trailing whitespace
1     W292 no newline at end of file
1     W605 invalid escape sequence '\.'
24

cidrblock · 2023-03-22T14:03:51Z

minor comment, isort is helpful to get consistency across projects for the import statement, this is the recommandation based on my local config


import argparse
import os
import re
import subprocess
import sys

from collections import defaultdict

import yaml

abikouo · 2023-03-22T14:45:16Z

I think this is fine, although in the case of a failure (sys.exit(1)) I'd like to see a bunch of debug level information printed out about what happened during the run. Doc strings, type hints, black and pylint/flake8 erros resolved. Here's some I found with a quick run:

validate_changelog.py:45:38: C0303: Trailing whitespace (trailing-whitespace)
validate_changelog.py:82:0: C0301: Line too long (111/100) (line-too-long)
validate_changelog.py:86:0: C0301: Line too long (104/100) (line-too-long)
validate_changelog.py:149:0: C0304: Final newline missing (missing-final-newline)
validate_changelog.py:15:47: W1401: Anomalous backslash in string: '\.'. String constant might be missing an r prefix. (anomalous-backslash-in-string)
validate_changelog.py:1:0: C0114: Missing module docstring (missing-module-docstring)
validate_changelog.py:14:0: C0116: Missing function or method docstring (missing-function-docstring)
validate_changelog.py:17:0: C0116: Missing function or method docstring (missing-function-docstring)
validate_changelog.py:40:0: C0116: Missing function or method docstring (missing-function-docstring)
validate_changelog.py:47:0: C0116: Missing function or method docstring (missing-function-docstring)
validate_changelog.py:50:7: R1729: Use a generator instead 'any(is_module_or_plugin(x) for x in changes['A'])' (use-a-generator)
validate_changelog.py:55:7: R1729: Use a generator instead 'all(is_documentation_file(x) for x in all_files)' (use-a-generator)
validate_changelog.py:60:0: C0116: Missing function or method docstring (missing-function-docstring)
validate_changelog.py:82:26: C0209: Formatting a regular string which could be a f-string (consider-using-f-string)
validate_changelog.py:86:24: C0209: Formatting a regular string which could be a f-string (consider-using-f-string)
validate_changelog.py:95:14: C0209: Formatting a regular string which could be a f-string (consider-using-f-string)
validate_changelog.py:98:0: C0116: Missing function or method docstring (missing-function-docstring)
validate_changelog.py:104:11: R1732: Consider using 'with' for resource-allocating operations (consider-using-with)
validate_changelog.py:108:0: C0116: Missing function or method docstring (missing-function-docstring)
validate_changelog.py:109:10: C0209: Formatting a regular string which could be a f-string (consider-using-f-string)
validate_changelog.py:110:10: C0209: Formatting a regular string which could be a f-string (consider-using-f-string)
validate_changelog.py:111:4: C0103: Variable name "rc" doesn't conform to snake_case naming style (invalid-name)
validate_changelog.py:117:8: C0103: Variable name "v" doesn't conform to snake_case naming style (invalid-name)
validate_changelog.py:122:0: C0116: Missing function or method docstring (missing-function-docstring)

validate_changelog.py:1:1: D100 Missing docstring in public module
validate_changelog.py:14:1: D103 Missing docstring in public function
validate_changelog.py:15:48: W605 invalid escape sequence '\.'
validate_changelog.py:17:1: D103 Missing docstring in public function
validate_changelog.py:17:1: E302 expected 2 blank lines, found 1
validate_changelog.py:40:1: D103 Missing docstring in public function
validate_changelog.py:45:39: W291 trailing whitespace
validate_changelog.py:47:1: D103 Missing docstring in public function
validate_changelog.py:47:1: E302 expected 2 blank lines, found 1
validate_changelog.py:60:1: D103 Missing docstring in public function
validate_changelog.py:60:1: E302 expected 2 blank lines, found 1
validate_changelog.py:82:101: E501 line too long (111 > 100 characters)
validate_changelog.py:86:101: E501 line too long (104 > 100 characters)
validate_changelog.py:95:84: E231 missing whitespace after ','
validate_changelog.py:98:1: D103 Missing docstring in public function
validate_changelog.py:98:1: E302 expected 2 blank lines, found 1
validate_changelog.py:108:1: D103 Missing docstring in public function
validate_changelog.py:108:1: E302 expected 2 blank lines, found 1
validate_changelog.py:122:1: D103 Missing docstring in public function
validate_changelog.py:122:1: E302 expected 2 blank lines, found 1
validate_changelog.py:130:80: E502 the backslash is redundant between brackets
validate_changelog.py:135:83: E502 the backslash is redundant between brackets
validate_changelog.py:142:1: E305 expected 2 blank lines after class or function definition, found 1
validate_changelog.py:149:39: W292 no newline at end of file
1     D100 Missing docstring in public module
8     D103 Missing docstring in public function
1     E231 missing whitespace after ','
6     E302 expected 2 blank lines, found 1
1     E305 expected 2 blank lines after class or function definition, found 1
2     E501 line too long (111 > 100 characters)
2     E502 the backslash is redundant between brackets
1     W291 trailing whitespace
1     W292 no newline at end of file
1     W605 invalid escape sequence '\.'
24

if all these check are performed we need to introduce them as a workflow for this repository

Qalthos · 2023-03-22T15:04:30Z

if all these check are performed we need to introduce them as a workflow for this repository

Yes, absolutely. This is the first Python file in the repo, so we should definitely be using this opportunity to set some checks.

.github/scripts/validate_changelog.py

abikouo · 2023-03-22T15:53:57Z

@Qalthos please try to suggest a commit and do not push the commit directly to the PR, otherwise, it is a pain to rebase it locally if there is a local modification

abikouo · 2023-03-22T15:58:05Z

@Qalthos why using another ci system for this repository instead of GitHub actions?

Qalthos · 2023-03-22T16:40:14Z

@Qalthos why using another ci system for this repository instead of GitHub actions?

Only because it was already set up, just not enforcing or checking Python files. It's not necessarily a complete or permanent solution, and the fact that it fails to autofix issues in workflows is an unexpected pain, so may require a revisit.

Qalthos

This adds mypy, flake8, and pylint checking, which should match @cidrblock's previous comment. Errors don't look too arduous to implement, but let me know if there you have any concerns.

mypy.ini

.pre-commit-config.yaml

Co-authored-by: Kate Case <[email protected]>

for more information, see https://pre-commit.ci

abikouo · 2023-03-23T15:08:23Z

@Qalthos pull request is ready for merge
Once merged, I can proceed with other pull request, Thx

Qalthos

Looks good otherwise.

scripts/validate_changelog.py

.pre-commit-config.yaml

Co-authored-by: Kate Case <[email protected]>

for more information, see https://pre-commit.ci

abikouo changed the title ~~DNM - add cloud detailed changelog~~ Enhance changelog job with more validation steps Mar 6, 2023

gravesm reviewed Mar 10, 2023

View reviewed changes

GomathiselviS reviewed Mar 10, 2023

View reviewed changes

.github/workflows/cloud-changelog.yml Outdated Show resolved Hide resolved

abikouo force-pushed the gha_cloud branch from cdfebf8 to 1863029 Compare March 13, 2023 09:50

abikouo requested review from GomathiselviS and gravesm and removed request for GomathiselviS and gravesm March 13, 2023 09:51

gravesm reviewed Mar 13, 2023

View reviewed changes

Qalthos approved these changes Mar 21, 2023

View reviewed changes

abikouo force-pushed the gha_cloud branch from 1863029 to 5880e30 Compare March 22, 2023 08:28

Qalthos self-requested a review March 22, 2023 14:13

Qalthos requested changes Mar 22, 2023

View reviewed changes

.github/scripts/validate_changelog.py Outdated Show resolved Hide resolved

abikouo and others added 3 commits March 22, 2023 11:21

update changelog workflow

34d9044

add the possibility to skip changelog validation

ce6ff12

pre-commit fixes

4b96b1a

Qalthos force-pushed the gha_cloud branch from 778830e to 4b96b1a Compare March 22, 2023 15:22

Qalthos and others added 4 commits March 22, 2023 11:35

Add black ans isort to pre-commit

ac8bbcd

add test workflow

a5711b6

add ci validation

0317a4a

rebase

18d3d55

move scripts from .github dir

89046e6

abikouo added 2 commits March 22, 2023 17:09

fix end-of-file linters

627b170

Remove github workflow

d875632

abikouo requested a review from Qalthos March 22, 2023 16:36

Qalthos requested changes Mar 22, 2023

View reviewed changes

mypy.ini Show resolved Hide resolved

.pre-commit-config.yaml Show resolved Hide resolved

abikouo and others added 2 commits March 22, 2023 18:41

Update mypy.ini

48d03be

Co-authored-by: Kate Case <[email protected]>

Update .pre-commit-config.yaml

055ce4c

Co-authored-by: Kate Case <[email protected]>

abikouo requested a review from Qalthos March 22, 2023 17:41

Fix linters issues

e194af2

abikouo force-pushed the gha_cloud branch from af6cc51 to e194af2 Compare March 23, 2023 15:06

[pre-commit.ci] auto fixes from pre-commit.com hooks

fea6201

for more information, see https://pre-commit.ci

abikouo added 2 commits March 23, 2023 16:32

fix script argument

4646737

add some debug information

a92c815

Qalthos requested changes Mar 23, 2023

View reviewed changes

scripts/validate_changelog.py Outdated Show resolved Hide resolved

scripts/validate_changelog.py Outdated Show resolved Hide resolved

.pre-commit-config.yaml Show resolved Hide resolved

abikouo and others added 4 commits March 24, 2023 08:58

Update .pre-commit-config.yaml

0c6565b

Co-authored-by: Kate Case <[email protected]>

Update scripts/validate_changelog.py

35142ca

Co-authored-by: Kate Case <[email protected]>

Update scripts/validate_changelog.py

e0e0186

Co-authored-by: Kate Case <[email protected]>

[pre-commit.ci] auto fixes from pre-commit.com hooks

37255e9

for more information, see https://pre-commit.ci

abikouo requested a review from Qalthos March 24, 2023 13:13

Qalthos approved these changes Mar 24, 2023

View reviewed changes

Qalthos merged commit 15d5f1b into ansible-network:main Mar 24, 2023

Enhance changelog job with more validation steps #44

Enhance changelog job with more validation steps #44

Uh oh!

Conversation

abikouo commented Feb 23, 2023

Uh oh!

gravesm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

abikouo commented Mar 13, 2023 • edited by Qalthos Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gravesm left a comment

Choose a reason for hiding this comment

Uh oh!

Qalthos left a comment

Choose a reason for hiding this comment

Uh oh!

cidrblock commented Mar 22, 2023

Uh oh!

cidrblock commented Mar 22, 2023

Uh oh!

abikouo commented Mar 22, 2023

Uh oh!

Qalthos commented Mar 22, 2023

Uh oh!

Uh oh!

abikouo commented Mar 22, 2023

Uh oh!

abikouo commented Mar 22, 2023

Uh oh!

Qalthos commented Mar 22, 2023

Uh oh!

Qalthos left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

abikouo commented Mar 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Qalthos left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

abikouo commented Mar 13, 2023 •

edited by Qalthos

Loading

abikouo commented Mar 23, 2023 •

edited

Loading