Add option to use hash in changelog filenames#996
Conversation
ecraig12345
force-pushed
the
ecraig/changelog-hash
branch
from
174f592 to
daa348b
Compare
derrickstolee
left a comment
derrickstolee
left a comment
There was a problem hiding this comment.
Thanks for working on this. I'm not an expert in this codebase or Javascript, so take all of my comments with a grain of salt. I tried to focus on the highest level of functionality, which I believe your implementation will suffice. I prod in a couple of directions for your consideration, but do not consider them blockers.
ecraig12345
changed the title
| if (existingSuffixedPaths[ext]) { | ||
| moveIfNeeded({ oldPath: existingSuffixedPaths[ext]!, newPath: defaultPath, hadSuffix: true }); | ||
| // Generate a unique filename based on the package name hash. | ||
| const hash = crypto.createHash('md5').update(packageName).digest('hex').slice(0, 8); |
There was a problem hiding this comment.
Thank you for this change. This will avoid the possibility of some corner cases with the random method. There may even be some benefits if a package is moved within the repo (without renaming the package).
| let newestDate = 0; | ||
| let newestFile: string | undefined; | ||
|
|
||
| for (const file of fs.readdirSync(cwd)) { |
There was a problem hiding this comment.
Best practice would be to pass the { withFileTypes: true } option and test that the entry is a file.
| if (fs.existsSync(oldPath)) { | ||
| fs.renameSync(oldPath, newPath); |
There was a problem hiding this comment.
Existence checking files before operating on them is generally an anti-pattern that is advised against by the authors of the file system APIs, since in theory the existence can change between the calls, so you have to handle the non-existence during the real call anyway. Simpler to just handle the ENOENT or ENOTDIR during the renameSync call.
ecraig12345
force-pushed
the
ecraig/changelog-hash
branch
from
f484507 to
474c58c
Compare
3 participants
Add an option
changelog.uniqueFilenameswhich adds an 8-characterrandomsuffix to the end of the changelog filename (before the extension): e.g.CHANGELOG-abcd1234.md.EDIT: After discussion, the suffix is now the first 8 characters of the MD5 hash digest of the package name.
This will stay stable between commits, and any existing changelog files will be renamed when modified. Existing suffixed files should also be renamed if the package is renamed. (I'm guessing 8 characters is good enough, but we could also use 12 or more.)
This is one option for working around an issue with Git: its default hash algorithm only considers the last 16 characters of filenames, which can lead to collisions and inefficient packing when many files have similar names, which then leads to extreme growth in size for larger repos with many colliding changelog files.
Bonus: while adding the hash, I learned we can use
crypto.randomUUID()for change file IDs and remove theuuiddependency.(I also updated a couple things with test configuration: disable type checking in ts-jest since it's redundant and slow, and set launch.json to use Node 14 like the rest of the repo.)
Related to #978