chore: write BCD version to README in web_generator#537
Conversation
Since it affects what we end up generating!
There was a problem hiding this comment.
Code Review
This pull request adds tracking for the @mdn/browser-compat-data package version in the README and updates the update_idl_bindings.dart script to automate this process. Review feedback suggests refactoring the version retrieval logic to avoid redundant parsing of the package-lock.json file and improving the log message to accurately reflect that all package versions are being updated.
|
|
||
| final sourceContent = readmeFile.readAsStringSync(); | ||
|
|
||
| final bcdVersion = _packageLockVersion(_browserCompatData); |
There was a problem hiding this comment.
The _packageLockVersion function reads and parses package-lock.json from disk every time it is called. With the addition of the BCD version, this file is now read and parsed four times in a row. Consider refactoring the logic to read and parse the lockfile once and then retrieve the versions for all required packages to improve efficiency.
| print(ansi.styleBold.wrap('No update for readme.')); | ||
| } else { | ||
| print(ansi.styleBold.wrap('Updating readme for IDL version $idlVersion')); | ||
| print(ansi.styleBold.wrap('Updating readme for IDL version $idlVersion and BCD version $bcdVersion')); |
There was a problem hiding this comment.
The log message specifically highlights the IDL and BCD versions, but the README update covers all four package versions (CSS, Elements, IDL, and BCD). If only the CSS or Elements versions change, this message could be misleading. Consider using a more inclusive message that reflects that all versions are being updated in the README.
| print(ansi.styleBold.wrap('Updating readme for IDL version $idlVersion and BCD version $bcdVersion')); | |
| print(ansi.styleBold.wrap('Updating readme with latest package versions')); |
Since it affects what we end up generating!