New: Use path to load resources in .sonarwhalrc
#1134
Conversation
|
Going to add tests and change some stuff in rule-runner, but you can take a look at this. If we are ok with this approach then #1125 can be closed. |
|
Pending things:
|
| @@ -244,11 +245,13 @@ const generateConfigPathsToResources = (configurations: Array<string>, name: str | |||
| */ | |||
| export const loadResource = (name: string, type: ResourceType, configurations: Array<string> = [], verifyVersion = false) => { | |||
| debug(`Searching ${name}…`); | |||
|
|
|||
| const isSource = fs.existsSync(name); // eslint-disable-line no-sync | |||
There was a problem hiding this comment.
if name is a relative path, this is not going to work, isn't it?
There was a problem hiding this comment.
It was working on my local tests. IIRC fs. exists uses process.cwd() so it should be ok?
I guess the problem is if we are calling from another place different place than where the .sonarwhalrc file is located (e.g.: when using the file in the user's home folder)
| const resourceName = nameSplitted[1] || packageName; | ||
| const resourceName = isSource ? | ||
| name : | ||
| nameSplitted[1] || packageName; | ||
|
|
||
| const key: string = `${type}-${name}`; |
There was a problem hiding this comment.
key should be something like
const key: string = isSource ? name : `${type}-${name}`
| @@ -322,7 +334,12 @@ export const loadResource = (name: string, type: ResourceType, configurations: A | |||
| throw new ResourceError(`Resource ${name} not found`, ResourceErrorStatus.NotFound); | |||
| } | |||
|
|
|||
| resources.set(key, resource); | |||
| if (isSource) { | |||
There was a problem hiding this comment.
You will no longer need this if you do the key change
| @@ -143,15 +143,15 @@ test.serial(`getRuleName - returns an empty string if it can't determine the rul | |||
| t.deepEqual(ruleName, ''); | |||
| }); | |||
|
|
|||
| test.serial(`getRuleName - returns a combined string if a package name is passed`, (t) => { | |||
| const sandbox = sinon.createSandbox(); | |||
| // test.serial(`getRuleName - returns a combined string if a package name is passed`, (t) => { | |||
There was a problem hiding this comment.
I have to rewrite all the getRuleName tests. None work after this and some of the previous logic didn't make any sense.
|
TL;DR: this is going to take longer than I expected if we want to do it right Longer version: |
|
New push that loads the path resources relatively to where the configuration file is. |
Enable loading a resource using a path instead of the ID of the package. This is useful when you have multiple configuration files with some common parts or when you have a locally developed rule that you don't want to publish on a package. - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - Fix #1123
Rename method and resolve relatively to the config - - - - - - - - - - - - - - - - Ref #1134
|
Closing in favor of #1140 that should be ready to review 😊 |
Enable loading a resource using a path instead of the ID of the package.
This is useful when you have multiple configuration files with some
common parts or when you have a locally developed rule that you don't
want to publish on a package.
Fix #1123
Pull request checklist
Make sure you:
For non-trivial changes, please make sure you also:
Short description of the change(s)