-
Notifications
You must be signed in to change notification settings - Fork 61
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: write metadata file #5875
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
luv 2 see it!
This pull request adds or modifies JavaScript ( |
@@ -38,7 +38,7 @@ const eventTriggeredFunctions = new Set([ | |||
'identity-login', | |||
]) | |||
|
|||
const validateCustomRoutes = function (functions: Awaited<ReturnType<typeof zipFunctions>>) { | |||
const validateCustomRoutes = function (functions: FunctionResult[]) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really related to the metadata file, but it's good to use the imported types from zip-it-and-ship-it.
|
||
import { execa } from 'execa' | ||
|
||
export const unzipFile = async function (path: string, dest: string): Promise<void> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've extracted this function from @netlify/zip-it-and-ship-it
so it can be used by any package in the monorepo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to update @netlify/zip-it-and-ship-it
to pull the function from @netlify/testing
, but sadly I'm getting a circular dependency error. For now, I'm keeping the function in both places.
@@ -138,16 +138,6 @@ export const unzipFiles = async function (files: FunctionResult[]): Promise<Test | |||
return files as TestFunctionResult[] | |||
} | |||
|
|||
const unzipFile = async function (path: string, dest: string): Promise<void> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved to @netlify/testing
.
|
|
||
t.is(semver.valid(metadata.bootstrap_version), metadata.bootstrap_version) | ||
t.is(metadata.branch, 'my-branch') | ||
t.is(metadata.version, 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: should we also delete the unzipped file? or will it get cleaned up eventually?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think these directories are being cleaned up at the moment. I think that's okay since in the context of an ephemeral CI it sometimes does more harm than good, as you need to spend time deleting a bunch of files. I think it would be nice to do it optionally for a local dev setup, but I'd rather handle that more holistically as part of the Fixture
class and not on this specific test.
Something for us to consider!
Summary
#5829 introduced a file in the bundle containing the version of the bootstrap layer. This PR replaces that with a more generic mechanism of writing different metadata properties at build time.
Part of https://linear.app/netlify/issue/RUN-1013/define-failure-mode-for-metadata-extraction.