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.

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

Resolve issues with rule 16 #52

Merged
merged 2 commits into from
Jul 6, 2024
Merged

Resolve issues with rule 16 #52

merged 2 commits into from
Jul 6, 2024

Conversation

SecondSkoll
Copy link
Contributor

Fix rule16 to reduce processing time on MD code blocks

Addresses #49

Fix rule16 to reduce processing time on MD code blocks
@SecondSkoll SecondSkoll changed the title Update 016-No-inline-comments.yml Resolve issues with rule 16 Jul 4, 2024
Copy link
Contributor

@ru-fu ru-fu left a comment

Choose a reason for hiding this comment

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

Could you explain what your trying to do in the regex? That would make it a lot easier to review and spot where it might go wrong.

Also, this rule won't catch code blocks that are created without explicit markup (through indentation).

styles/Canonical/016-No-inline-comments.yml Outdated Show resolved Hide resolved
@izmalk
Copy link
Contributor

izmalk commented Jul 4, 2024

I can confirm that this change solves an issue of Vale hanging when trying to process something in the https://github.com/canonical/kafka-operator docs. With the fix, vale produces the result quite fast.

Fix additional processing issue with very long MD files by limiting rules to single code blocks.
@SecondSkoll
Copy link
Contributor Author

Processing issue seems to have been resolved. Using time make vale now results in:

real    0m1.352s
user    0m1.313s
sys     0m0.160s

Rather than having to stop it after 20+ minutes:

KeyboardInterrupt
make[1]: *** [Makefile.sp:115: sp-vale] Interrupt
make: *** [Makefile:30: vale] Interrupt

real    22m53.559s
user    22m54.040s
sys     0m1.188s

Copy link
Contributor

@ru-fu ru-fu left a comment

Choose a reason for hiding this comment

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

I'm okay with the changes to fix/unblock the current processing, but the rules will need to be revisited.

Copy link
Contributor

@evilnick evilnick left a comment

Choose a reason for hiding this comment

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

👍

@evilnick evilnick merged commit 14f19bc into canonical:main Jul 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants