doc, tools: added caching support for CHANGELOG.md in tools/doc/versions.js
#32515
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ons.js When running `make doc` the CHANGELOG.md file is pulled everytime for each of the doc file that requires the versions list. This commit introduces a new file called `.master-CHANGELOG.md` which will be created once so that subsequent calls for the version are pulled locally instead of from the repo. Since this file is not part of the codebase proper, it is appended in the .gitignore file. References Issue nodejs#32512
nodejs-github-bot
added
doc
himself65
reviewed
Mar 27, 2020
tools/doc/versions.js
Outdated
| }); | ||
| }; | ||
|
|
||
| const createMaster = (masterPath, file) => { |
There was a problem hiding this comment.
this function only is used once, why not code inline?
There was a problem hiding this comment.
You are right. My bad.
Thanks for the suggestion.
…angelog Review for the pull request nodejs#32515 suggested to use inline command instead of creating a function that is not used again.
addaleax
reviewed
Mar 27, 2020
| const masterChangelog = path.join(srcRoot, '.master-CHANGELOG.md'); | ||
| if (kNoInternet) { | ||
| changelog = readFileSync(file, { encoding: 'utf8' }); | ||
| } else if (existsSync(masterChangelog)) { |
There was a problem hiding this comment.
This should probably be using statSync and check whether the file is outdated – otherwise this isn’t caching, it’s just storing a file and keeping it around forever.
(Also, it’s odd that we use fs.*Sync() functions inside an async function, although it’s not that important for this script. I’d suggest using readFile, stat, writeFile from fs.promises instead, just to reflect best practices.)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When running
make docthe CHANGELOG.md file is pulled everytime for eachof the doc file that requires the versions list. This commit introduces a
new file called
.master-CHANGELOG.mdwhich will be created once so thatsubsequent calls for the version are pulled locally instead of from the repo.
Since this file is not part of the codebase proper, it is appended in the
.gitignore file.
Closes #32512
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes