only copy theme resources when they actually change #990
Conversation
| // we check for an exact match on the timestamp. We use seconds instead | ||
| // of ms because utimesSync doesn't set the times with ms precision | ||
| if (!fs.existsSync(destinationFile) || | ||
| Math.floor(new Date(fs.statSync(sourceFile).mtimeMs).getTime() / 1000) != |
There was a problem hiding this comment.
ideally we'd use a file hash here, but that might take a while
lib/util.js
Outdated
|
|
||
| if (process.platform === 'darwin') { | ||
| // Copy proper mac app icon for channel to chrome/app/theme/mac/app.icns. | ||
| // Each channel's app icons are stored in brave/app/theme/$channel/app.icns. | ||
| // With this copying, we don't need to modify chrome/BUILD.gn for this. | ||
| fs.copySync(path.join(braveAppDir, 'theme', 'brave', 'mac', config.channel, 'app.icns'), | ||
| path.join(chromeAppDir, 'theme', 'brave', 'mac', 'app.icns')) | ||
| fileMap[path.join(braveAppDir, 'theme', 'brave', 'mac', config.channel, 'app.icns')] = |
There was a problem hiding this comment.
I think these two files (icon and BRANDING) should be handled differently.
When different channel is built from previous, these two files should be copied.
This kinds of file could be candidates for file hash check.
There was a problem hiding this comment.
it should still work because I'm checking the actual timestamp and not just checking to see which one is newer
There was a problem hiding this comment.
There was a problem hiding this comment.
I guess it could still be a problem if you switch channels back and forth because you'll end up with the same timestamp for all of them at some point
There was a problem hiding this comment.
I'll change to hash
only copy theme resources when they actually change
|
0.55.x ac51cfb |
ghost
mentioned this pull request
fix #989
Submitter Checklist:
npm test brave_unit_tests && npm test brave_browser_tests) ongit rebase master(if needed).git rebase -ito squash commits (if needed).Test Plan:
Reviewer Checklist: