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

feat: refactoring and tests #164

Merged
merged 8 commits into from
Jan 28, 2022

Conversation

themr0c
Copy link
Collaborator

@themr0c themr0c commented Jan 26, 2022

TLDR: Fewer rules, all tested. Reference guide less overwhelming.

feat: consolidate the rules in fewer files (25 rules rather than 295)
feat: remove rules that are not defined in supplementary style guide or underlying style guides
feat: order fields consistently in rules
feat: point link: in rules to vale-at-red-hat docs
feat: point source: in rules to supplementary style guide
feat: create tools/generate_vale_rule_tests.sh
feat: create tools/test_vale_rules.sh
feat: generate vale rule tests
feat: generate reference guide and vale rule tests during preview
feat: refactor build script
feat: author extensive tests for all rules
feat: all rules pass the tests

@github-actions
Copy link

Click here to review and test in web IDE: Contribute

@themr0c themr0c force-pushed the feat-rules-consolidate branch 3 times, most recently from 7c3f041 to 5cf3b0a Compare January 26, 2022 16:29
@themr0c
Copy link
Collaborator Author

themr0c commented Jan 26, 2022

Status: The alerts are not the same (testing che-docs master branch). Need more in depth tests.

main:

✖ 128 errors, 502 warnings and 3200 suggestions in 764 files.

this branch:

✖ 217 errors, 40 warnings and 9769 suggestions in 764 files.

@themr0c themr0c force-pushed the feat-rules-consolidate branch 2 times, most recently from 5f68140 to c2b5bb7 Compare January 26, 2022 16:49
@themr0c themr0c force-pushed the feat-rules-consolidate branch 3 times, most recently from 9589230 to 26dd193 Compare January 27, 2022 10:35
@themr0c themr0c changed the title feat: consolidate the rules in fewer files feat: refactoring and tests Jan 27, 2022
@themr0c themr0c force-pushed the feat-rules-consolidate branch 2 times, most recently from d2cbfa6 to 89a7672 Compare January 27, 2022 11:45
feat: test suite generator update_test_suite
feat: generate test suites
feat: run update_reference_guide and update_test_suite during preview
feat: umask 002 to avoid issues with rights on new files
feat: add tests for all rules
feat: fix rules using tests
@themr0c
Copy link
Collaborator Author

themr0c commented Jan 27, 2022

After updates: testing che-docs

  • main: ✖ 3 errors, 502 warnings and 3230 suggestions in 764 files.
  • this branch: ✖ 2 errors, 33 warnings and 3625 suggestions in 764 files.

The rules in this branch are currently better than the rules in the main branch.

@themr0c themr0c force-pushed the feat-rules-consolidate branch 2 times, most recently from 72bf5a3 to 9713959 Compare January 27, 2022 15:58
@themr0c
Copy link
Collaborator Author

themr0c commented Jan 27, 2022

Now we know where we stand:

Run tools/test_vale_rules.sh
0 tests to fix

@themr0c themr0c force-pushed the feat-rules-consolidate branch 2 times, most recently from 1cb4b38 to d9be877 Compare January 27, 2022 22:26
feat: refactoring rules further
feat: all rules pass the tests
@themr0c
Copy link
Collaborator Author

themr0c commented Jan 27, 2022

I have some doubt about raising an error for any occurrence of which not preceded by a comma.

Copy link
Member

@aireilly aireilly left a comment

Choose a reason for hiding this comment

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

LGTM

feat: alphabetical ordering in test files
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants