ci: set check_latest to true #363
Conversation
github-actions
bot
added
the
type: chore
|
Windows tests are failing on Node.js 16, looking into the failures and it suggests the plugin is not compatible with Node.js 16 + Windows |
erezrokah
force-pushed
the
chore/fix_check_latest_flag
branch
from
a8df756 to
502ec5e
Compare
erezrokah
force-pushed
the
chore/fix_check_latest_flag
branch
from
502ec5e to
9916c61
Compare
| @@ -22,15 +22,13 @@ const handleFileTracking = ({ functionsPath, publishPath }) => { | |||
| const trackingFile = readFileSync(trackingFilePath, 'utf8') | |||
| const [trackedFunctions, trackedPublish] = trackingFile.split(TRACKING_FILE_SEPARATOR) | |||
| const cleanConfiguredFiles = (trackedFiles, dirPath) => { | |||
| trackedFiles.forEach((file) => { | |||
| const filePath = join(dirPath, file.trim('\r')) | |||
There was a problem hiding this comment.
Not sure I understand the original code. trim doesn't accept an argument. Also recursive: true is not supported for Node.js 10.
Finally I'm not sure why we need rmdirSync on Windows since removeSync does the same without failing on non existent files.
There was a problem hiding this comment.
basically this work was trial and error on windows when i first did it. it was just to get a bandaid solution out so plugin users could even SORTOF use the CLI, in lieu of a CLI-based solution like having plugins build into a sep dir (instead of changing their src). if all the tests pass still with this diff then thats great!!
There was a problem hiding this comment.
Sorry I should have been more clear. trim('\r') is the same as trim() as trim doesn't accept an argument.
| @@ -22,15 +22,13 @@ const handleFileTracking = ({ functionsPath, publishPath }) => { | |||
| const trackingFile = readFileSync(trackingFilePath, 'utf8') | |||
| const [trackedFunctions, trackedPublish] = trackingFile.split(TRACKING_FILE_SEPARATOR) | |||
| const cleanConfiguredFiles = (trackedFiles, dirPath) => { | |||
| trackedFiles.forEach((file) => { | |||
| const filePath = join(dirPath, file.trim('\r')) | |||
| if (file !== '') { | |||
There was a problem hiding this comment.
I'm not sure but the intention here might be to check the trimmed version of file
There was a problem hiding this comment.
trackingFile.split would sometimes insert '' into trackedFiles; this is just to skip that
There was a problem hiding this comment.
in these cases, with file === '', it removes the current dir
| if (process.platform === 'win32') { | ||
| rmdirSync(filePath, { recursive: true }) | ||
| } | ||
| trackedFiles |
There was a problem hiding this comment.
@lindsaylevine can you approve this change?
| @@ -22,15 +22,13 @@ const handleFileTracking = ({ functionsPath, publishPath }) => { | |||
| const trackingFile = readFileSync(trackingFilePath, 'utf8') | |||
| const [trackedFunctions, trackedPublish] = trackingFile.split(TRACKING_FILE_SEPARATOR) | |||
| const cleanConfiguredFiles = (trackedFiles, dirPath) => { | |||
| trackedFiles.forEach((file) => { | |||
| const filePath = join(dirPath, file.trim('\r')) | |||
There was a problem hiding this comment.
basically this work was trial and error on windows when i first did it. it was just to get a bandaid solution out so plugin users could even SORTOF use the CLI, in lieu of a CLI-based solution like having plugins build into a sep dir (instead of changing their src). if all the tests pass still with this diff then thats great!!
| @@ -22,15 +22,13 @@ const handleFileTracking = ({ functionsPath, publishPath }) => { | |||
| const trackingFile = readFileSync(trackingFilePath, 'utf8') | |||
| const [trackedFunctions, trackedPublish] = trackingFile.split(TRACKING_FILE_SEPARATOR) | |||
| const cleanConfiguredFiles = (trackedFiles, dirPath) => { | |||
| trackedFiles.forEach((file) => { | |||
| const filePath = join(dirPath, file.trim('\r')) | |||
| if (file !== '') { | |||
There was a problem hiding this comment.
trackingFile.split would sometimes insert '' into trackedFiles; this is just to skip that
| @@ -22,15 +22,13 @@ const handleFileTracking = ({ functionsPath, publishPath }) => { | |||
| const trackingFile = readFileSync(trackingFilePath, 'utf8') | |||
| const [trackedFunctions, trackedPublish] = trackingFile.split(TRACKING_FILE_SEPARATOR) | |||
| const cleanConfiguredFiles = (trackedFiles, dirPath) => { | |||
| trackedFiles.forEach((file) => { | |||
| const filePath = join(dirPath, file.trim('\r')) | |||
| if (file !== '') { | |||
There was a problem hiding this comment.
in these cases, with file === '', it removes the current dir
| } | ||
| trackedFiles | ||
| .map((file) => file.trim()) | ||
| .filter(Boolean) |
There was a problem hiding this comment.
is this effectively the same thing as if (file !== '') ?
There was a problem hiding this comment.
In the original code if (file !== '') is the original non trimmed value (trim doesn't mutate the string), so the check should be if (file.trim() !== '').
filter(Boolean) operates on the trimmed value.
|
Thanks for the review @lindsaylevine |
* fix: import server in module scope * chore: rmeove comment
Follow up on #328