Adding sync up script to add data here #2
Conversation
| const pkg = content.module_name; | ||
|
|
||
| const targetPath = path.join(NPM_TARGET_ROOT, pkg); | ||
| if(!fs.existsSync(targetPath)) { |
There was a problem hiding this comment.
We shouldn't check before creating, right?
There was a problem hiding this comment.
I'm not sure I get this comment :/
There was a problem hiding this comment.
Sorry, I should have explained myself better there. Before creating a file or directory we should not check if it exists. We should just try to create. If it fails, it's already there. That would be more reliable, I believe.
There was a problem hiding this comment.
I am not sure, I might have to add vulnerabilities to a dir already existing
tools/sync_up.js
Outdated
| if(!fs.existsSync(targetPath)) { | ||
| fs.mkdirSync(targetPath); | ||
| } | ||
| if (!fs.existsSync(path.join(targetPath, vulnname))) { |
There was a problem hiding this comment.
Is the intention not to overwrite the file?
There was a problem hiding this comment.
Yes, but thinking of it, we should indeed override the file
tools/sync_up.js
Outdated
|
|
||
| // collect and set up ecosystem reports | ||
| fs.readdirSync(path.join(SWG_ROOT, 'npm')) | ||
| .forEach((vulnname) => { |
There was a problem hiding this comment.
This fits on the previous line and is still under 80 characters.
There was a problem hiding this comment.
Colin are you editing on tmux? ;-)
tools/sync_up.js
Outdated
| const CORE_TARGET_ROOT = path.join(__dirname, '..', 'core'); | ||
|
|
||
| // collect and set up ecosystem reports | ||
| fs.readdirSync(path.join(SWG_ROOT, 'npm')) |
There was a problem hiding this comment.
Maybe store this join() result in a variable since it's also part of the join() below.
| .forEach((vulnname) => { | ||
|
|
||
| const rawContent = fs.readFileSync(path.join(SWG_ROOT, 'npm', vulnname)); | ||
| const content = JSON.parse(rawContent); |
There was a problem hiding this comment.
This can throw, but I guess we should probably crash in that case anyway.
tools/sync_up.js
Outdated
| const fs = require('fs'); | ||
| const path = require('path'); | ||
|
|
||
| const SWG_ROOT = path.join(__dirname, '..', 'node_modules', 'security-wg', 'vuln'); |
There was a problem hiding this comment.
can we rename the var to make it easy to understand that this is actually the current original repo? something like maybe SWG_ROOT_SRC or something else.
tools/sync_up.js
Outdated
|
|
||
| // collect and set up ecosystem reports | ||
| fs.readdirSync(path.join(SWG_ROOT, 'npm')) | ||
| .forEach((vulnname) => { |
There was a problem hiding this comment.
Colin are you editing on tmux? ;-)
| fs.writeFileSync(path.join(targetPath, vulnname), rawContent); | ||
| }); | ||
|
|
||
| // same for core vulns |
There was a problem hiding this comment.
we could probably have just one function that does this and we give it different dirs as source and target so we keep the code simpler and smaller.
There was a problem hiding this comment.
Not sure; the logic is not fully similar here (create subdirs or not)
No description provided.