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

Jump to bottom

add support for inheriting rulesets #207

Merged
merged 2 commits into from
Apr 14, 2021
Merged

add support for inheriting rulesets #207

merged 2 commits into from
Apr 14, 2021

Conversation

willnorris
Copy link
Member

@willnorris willnorris commented Mar 30, 2021 •
edited
Loading

This adds support for inheriting rulesets with a new extends property:

"$schema": "https://raw.githubusercontent.com/todogroup/repolinter/master/rulesets/schema.json"
version: 2
extends: "https://raw.githubusercontent.com/todogroup/repolinter/master/rulesets/default.json"
rules:
  license-file-exists:
    level: off

The full parsed config files are merged using lodash.merge, with the "current" file overwriting the "extended" file. This allows adding new rules, disabling rules by setting level: off, or customizing options for a rule (though the way lodash merges values may not always be what you expect... it requires a little experimenting).

This PR contains two commits... the first just refactors the existing config loading logic, which was previously spread out in a couple of places. It does have very minimal changes to existing behavior, mostly in some error messages. The second commit adds the extending logic.

This is not ready to be merged yet, as I still need to add some tests and docs, but I'd like to get the general approach reviewed. I've also hardcoded the max number of extended rulesets to 20 for now just to prevent accidental infinite loops. We could expose that as a flag at some point if someone really needed more than that, but that seems unlikely.

(Supporting the inclusion of config snippets inline, rather than extending an entire file, similar to jsonschema's $ref properties is also something I'm thinking about. But that is intentionally excluded from this PR).

@willnorris willnorris force-pushed the config branch 2 times, most recently from 7640344 to 4b9610c Compare March 31, 2021 03:12
prototypicalpro
prototypicalpro requested changes Mar 31, 2021
View reviewed changes
Copy link
Collaborator

@prototypicalpro prototypicalpro left a comment

Choose a reason for hiding this comment

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

Love the idea, thanks for implementing it! I like the general approach (a deep merge should cover most use cases), and I left a few comments on the internal logic. Let me know if you have any questions. I'd also be happy to implement the changes I suggested if that would be useful.

lib/config.js Outdated Show resolved Hide resolved
lib/config.js Show resolved Hide resolved
bin/repolinter.js Outdated Show resolved Hide resolved
lib/config.js Outdated Show resolved Hide resolved
lib/config.js Show resolved Hide resolved
@willnorris
Copy link
Member Author

I'm stumped as to why tests are failing on macos now, but I'll have to look at it later in the week. I still need to add user docs and tests for new functionality, but I think we're getting closer now.

@prototypicalpro
Copy link
Collaborator

It looks like github_markup isn't being resolved? I don't think that's your changes, weird that it's only happening on mac.

@willnorris
Copy link
Member Author

what's super weird now is that tests pass on one branch, but not another, even with the exact same commit! Super weird. I'm wondering if it has to do with a bad cache that's getting in the way? Though it doesn't look like our cache keys include the branch name, so I'm not sure that that would make a difference.

@willnorris
Copy link
Member Author

Just added tests and docs, so I think this should be ready for a final review. I may rebase onto master, and potentially merge some of the commits, but wanted to leave it as-is for now for you to review.

@willnorris willnorris marked this pull request as ready for review April 8, 2021 00:43
@willnorris willnorris requested a review from trevmex as a code owner April 8, 2021 00:43
@willnorris
Copy link
Member Author

(oops... just noticed I don't have a test for circular extensions. Will add that shortly)

mynameistechno
mynameistechno approved these changes Apr 14, 2021
View reviewed changes
Copy link
Contributor

@mynameistechno mynameistechno left a comment

Choose a reason for hiding this comment

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

Took a quick look and LGTM. Nice clean up with the config module. I will try to pull it down and run a test.

prototypicalpro
prototypicalpro approved these changes Apr 14, 2021
View reviewed changes
Copy link
Collaborator

@prototypicalpro prototypicalpro left a comment

Choose a reason for hiding this comment

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

Looks good to me, sorry for the delay. Thanks for the PR!

@prototypicalpro
Copy link
Collaborator

Before this merges I will see if I can fix the MacOS CI so the change is released properly,

@willnorris
Copy link
Member Author

now that reviews are done, I'll rebase this and merge a couple of commits. Maybe that will unstick the macos issue as well?

@willnorris
refactor configuration loading logic
67ea12f 
Currently, logic for loading configuration files is split between
bin/repolinter.js (for loading URLs) and index.js (for local files,
directories, and the default config).

This commit refactors all configuration loading logic into a new
lib/config.js module, which handles loading and validating config files
of all types (existing parseConfig and validateConfig functions were
moved as-is).  This will make it simpler to extend config loading logic,
for example to support inheriting rulesets (todogroup#21).

This does have minor changes on the error messages returned by the CLI
in some cases.  It also attempts to parse all config files as JSON
first and then YAML, regardless of file extensions (this was previously
different for remote versus local config files).

This does not change anything in repolinter's programmatic API.
@prototypicalpro
Copy link
Collaborator

I think #213 fixed the issue, if you rebase it should be working now.

@willnorris
Copy link
Member Author

(sorry... ignore that push. About to do the correct one)

@willnorris
Copy link
Member Author

okay, assuming this latest rebase passes the tests, we should be good to go (I hope).

@willnorris
Copy link
Member Author

woohoo! Tests are happy now!

@willnorris
Copy link
Member Author

though interestingly, this isn't fully rebased onto master. :-\

@prototypicalpro prototypicalpro merged commit ca1ae01 into todogroup:master Apr 14, 2021
github-actions bot pushed a commit that referenced this pull request Apr 14, 2021
@semantic-release-bot
chore(release): 0.11.0
b6e8287 
# [0.11.0](v0.10.1...v0.11.0) (2021-04-14)

### Features

* add support for inheriting rulesets ([#207](#207)) ([ca1ae01](ca1ae01)), closes [#21](#21)
@github-actions
Copy link

🎉 This PR is included in version 0.11.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@willnorris willnorris deleted the config branch April 14, 2021 19:30
@prototypicalpro prototypicalpro mentioned this pull request Apr 22, 2021
Closed
@willnorris willnorris mentioned this pull request Jul 9, 2021
Closed
github-actions bot pushed a commit to newrelic-forks/repolinter that referenced this pull request Feb 17, 2023
@semantic-release-bot
chore(release): 0.9.0
93fb7b2 
# [0.9.0](v0.8.2...v0.9.0) (2023-02-17)

### Bug Fixes

* add backwards compatibility for blacklist -> denylist switch ([002deae](002deae))
* add command-exists test to axioms for stability, moved from sync to async calls ([todogroup#182](https://github.com/newrelic-forks/repolinter/issues/182)) ([0da11b9](0da11b9))
* add missing github-markup dependencies to dockerfile ([b5fb88a](b5fb88a))
* attempt to fix timeout on MacOS when running axioms ([05e81ad](05e81ad))
* broken test case. ([4816a53](4816a53))
* explicitly check-out the forked repo for release job ([5c7d05b](5c7d05b))
* file-hash now accepts legacy configuration format ([d509274](d509274))
* fix axiom object with invalid axiom ([11e7048](11e7048))
* fix broken link behavrior with files in subdirectories ([6c14db9](6c14db9))
* fix flaky glob behavior with broken symlinks ([57c0dfd](57c0dfd))
* fix path seperator in symlink detection ([3898ed5](3898ed5))
* fix pathing issues and succeed/fail criteria with no files found ([c0c101b](c0c101b))
* jsdoc initial rules ([d200c51](d200c51))
* minor updates to the markdown formatter ([todogroup#180](https://github.com/newrelic-forks/repolinter/issues/180)) ([45ee596](45ee596))
* move command-exists to dependencies ([72abd8a](72abd8a))
* remove accidental shadowing of fs ([d5d6359](d5d6359))
* remove lstat check, fix flakey fs tests ([a154a89](a154a89))
* remove npm from plugins ([a628f6c](a628f6c))
* remove Object.fromEntries for node 10 support ([0644374](0644374))
* schema $id mixup. ([6bc0828](6bc0828))
* strip quotes from stringified content ([5222510](5222510))
* switch to canonical broken-link-checker ([todogroup#205](https://github.com/newrelic-forks/repolinter/issues/205)) ([e62d767](e62d767))
* undo change ([cd23a3a](cd23a3a))
* undo change ([33d6384](33d6384))
* update action to semantic-relase v19.0.5 is used ([b043a24](b043a24))
* update axioms, fixes and rules for documentation ([7432dfe](7432dfe))
* update base image ([1d0a938](1d0a938))
* update dockerfile to fix bad copying ([todogroup#224](https://github.com/newrelic-forks/repolinter/issues/224)) ([92389be](92389be))
* update dockerfile to reconfigure bundle which (may) have caused some bugs on linux ([889da3e](889da3e))
* update github-markup to remove python2 dependency ([todogroup#209](https://github.com/newrelic-forks/repolinter/issues/209)) ([0c7c50c](0c7c50c))
* update JSON schema URLs to point to TODOGroup repo ([9d397ef](9d397ef))
* update test case for 'fail-on-non-existent' scenario, the 'passed' of non-exist file should depend on the parameter. ([e31bfb3](e31bfb3))
* upgrade broken-link-checker to add node 10 support ([4f00b33](4f00b33))
* upgrade ruby gems to latest version ([e36c10a](e36c10a))
* windows compatibility fixes ([8fc0540](8fc0540))
* windows fixes ([d52353e](d52353e))

### Features

* add 'files-not-contents' rule to make string detection rulesets easier to maintain. ([093afc3](093afc3))
* add better regex context support for file-contents ([2ff65d1](2ff65d1))
* add file-not-exists rule ([184b787](184b787))
* add file-not-exists rule ([todogroup#183](https://github.com/newrelic-forks/repolinter/issues/183)) ([ebca0b1](ebca0b1))
* add file-remove fix ([04743ce](04743ce))
* add file-remove fix ([todogroup#181](https://github.com/newrelic-forks/repolinter/issues/181)) ([ee913b3](ee913b3))
* add files-not-hash rule for files detections. ([d2f262a](d2f262a))
* add large file rules ([2e48a85](2e48a85))
* add lines of regex match in the file-contents and file-not-contents rule output, default turned off. ([9831684](9831684))
* add policyUrl and policyInfo in the default output. ([fb81866](fb81866))
* add support for inheriting rulesets ([todogroup#207](https://github.com/newrelic-forks/repolinter/issues/207)) ([ca1ae01](ca1ae01)), closes [todogroup#21](https://github.com/newrelic-forks/repolinter/issues/21)
* add YAML support in config ([fb6c743](fb6c743))
* finalize no broken links rule and dockerfile ([c1b1f72](c1b1f72))
* merge contributor-count axiom into fork ([f3dc857](f3dc857))
* **no-broken-links:** add option to pass or not pass in external links ([aaa92f8](aaa92f8))
* switch to fork of broken-link-checker ([7df3086](7df3086))
* update dockerfile to reflect new dependencies ([c256af7](c256af7))
* WIP adding a broken link checker rule ([9e8bb98](9e8bb98))
github-actions bot pushed a commit to zhaoyuheng200/repolinter that referenced this pull request Aug 17, 2024
@semantic-release-bot
chore(release): 1.0.0
287a945 
# 1.0.0 (2024-08-17)

### Bug Fixes

* add backwards compatibility for blacklist -> denylist switch ([002deae](002deae))
* add command-exists test to axioms for stability, moved from sync to async calls ([todogroup#182](https://github.com/zhaoyuheng200/repolinter/issues/182)) ([0da11b9](0da11b9))
* add docs directory ([0ccdcd8](0ccdcd8))
* add missing github-markup dependencies to dockerfile ([b5fb88a](b5fb88a))
* attempt to fix timeout on MacOS when running axioms ([05e81ad](05e81ad))
* broken test case. ([4816a53](4816a53))
* **code-of-conduct:** match CODE_OF_CONDUCT* as well ([8892334](8892334))
* **dependencies:** update vulnerable packages ([#1](#1)) ([28a6135](28a6135)), closes [/github.com/sinonjs/sinon/blob/master/CHANGELOG.md#412--2017-11-07](https://github.com//github.com/sinonjs/sinon/blob/master/CHANGELOG.md/issues/412--2017-11-07)
* file-hash now accepts legacy configuration format ([d509274](d509274))
* fix axiom object with invalid axiom ([11e7048](11e7048))
* fix broken link behavrior with files in subdirectories ([6c14db9](6c14db9))
* fix flaky glob behavior with broken symlinks ([57c0dfd](57c0dfd))
* fix path seperator in symlink detection ([3898ed5](3898ed5))
* fix pathing issues and succeed/fail criteria with no files found ([c0c101b](c0c101b))
* jsdoc initial rules ([d200c51](d200c51))
* **licensee:** isWindows is a function ([09a772b](09a772b))
* make license-headers-exist reuse.software compatible ([3739198](3739198))
* minor updates to the markdown formatter ([todogroup#180](https://github.com/zhaoyuheng200/repolinter/issues/180)) ([45ee596](45ee596))
* move command-exists to dependencies ([72abd8a](72abd8a))
* remove accidental shadowing of fs ([d5d6359](d5d6359))
* remove lstat check, fix flakey fs tests ([a154a89](a154a89))
* remove Object.fromEntries for node 10 support ([0644374](0644374))
* schema $id mixup. ([6bc0828](6bc0828))
* switch to canonical broken-link-checker ([todogroup#205](https://github.com/zhaoyuheng200/repolinter/issues/205)) ([e62d767](e62d767))
* undo change ([cd23a3a](cd23a3a))
* undo change ([33d6384](33d6384))
* update axioms, fixes and rules for documentation ([7432dfe](7432dfe))
* update dockerfile to fix bad copying ([todogroup#224](https://github.com/zhaoyuheng200/repolinter/issues/224)) ([92389be](92389be))
* update dockerfile to reconfigure bundle which (may) have caused some bugs on linux ([889da3e](889da3e))
* update github-markup to remove python2 dependency ([todogroup#209](https://github.com/zhaoyuheng200/repolinter/issues/209)) ([0c7c50c](0c7c50c))
* update JSON schema URLs to point to TODOGroup repo ([9d397ef](9d397ef))
* update test case for 'fail-on-non-existent' scenario, the 'passed' of non-exist file should depend on the parameter. ([e31bfb3](e31bfb3))
* upgrade broken-link-checker to add node 10 support ([4f00b33](4f00b33))
* upgrade ruby gems to latest version ([e36c10a](e36c10a))
* windows compatibility fixes ([8fc0540](8fc0540))
* windows fixes ([d52353e](d52353e))

### Features

* add 'files-not-contents' rule to make string detection rulesets easier to maintain. ([093afc3](093afc3))
* add better regex context support for file-contents ([2ff65d1](2ff65d1))
* add file-not-exists rule ([184b787](184b787))
* add file-not-exists rule ([todogroup#183](https://github.com/zhaoyuheng200/repolinter/issues/183)) ([ebca0b1](ebca0b1))
* add file-remove fix ([04743ce](04743ce))
* add file-remove fix ([todogroup#181](https://github.com/zhaoyuheng200/repolinter/issues/181)) ([ee913b3](ee913b3))
* add files-not-hash rule for files detections. ([d2f262a](d2f262a))
* add large file rules ([2e48a85](2e48a85))
* add lines of regex match in the file-contents and file-not-contents rule output, default turned off. ([9831684](9831684))
* add policyUrl and policyInfo in the default output. ([fb81866](fb81866))
* add relative path resolving to rulesetPath ([561e895](561e895))
* add support for inheriting rulesets ([todogroup#207](https://github.com/zhaoyuheng200/repolinter/issues/207)) ([ca1ae01](ca1ae01)), closes [todogroup#21](https://github.com/zhaoyuheng200/repolinter/issues/21)
* add YAML support in config ([fb6c743](fb6c743))
* finalize no broken links rule and dockerfile ([c1b1f72](c1b1f72))
* merge contributor-count axiom into fork ([f3dc857](f3dc857))
* **no-broken-links:** add option to pass or not pass in external links ([aaa92f8](aaa92f8))
* switch to fork of broken-link-checker ([7df3086](7df3086))
* update dockerfile to reflect new dependencies ([c256af7](c256af7))
* update github pull request template locations ([c2d0f40](c2d0f40))
* WIP adding a broken link checker rule ([9e8bb98](9e8bb98))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@prototypicalpro prototypicalpro prototypicalpro approved these changes

@mynameistechno mynameistechno mynameistechno approved these changes

@iamwillbar iamwillbar Awaiting requested review from iamwillbar

@trevmex trevmex Awaiting requested review from trevmex

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@willnorris @prototypicalpro @mynameistechno