EVG-20677 & EVG-20868: Make type.ts validation more robust #2038
Conversation
Passing run #12749 ↗︎
Details:
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. |
||||||||||||||||||||
| @@ -3,35 +3,35 @@ import path from "path"; | |||
|
|
|||
| export const getConfig = ({ | |||
There was a problem hiding this comment.
Sorry I know this was here but do you know why this is exported twice?
There was a problem hiding this comment.
The first export is the util function and the second export is the output of the util function. Although it's not stated explicitly in the docs graphql-codegen --config codegen.ts needs a default export from codegen.ts with the parameters applied.
| /** | ||
| * Checks if a given domain can be resolved. | ||
| * @async | ||
| * @param domain - The domain name to check. | ||
| * @returns - Resolves to `true` if the domain can be resolved, `false` otherwise. | ||
| */ | ||
| export const canResolveDNS = (domain: string) => | ||
| new Promise((resolve) => { | ||
| dns.lookup(domain, (err) => { | ||
| if (err) { | ||
| resolve(false); | ||
| } else { | ||
| resolve(true); | ||
| } | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Is this necessary? We can avoid this request by just checking if the fetch request returned the same error.
| * @returns A Promise that resolves to the SHA of the latest commit. | ||
| * @throws {Error} When failed to fetch commits. | ||
| */ | ||
| export async function getLatestCommitFromRemote(): Promise<string> { |
There was a problem hiding this comment.
Can we use arrow syntax for consistency?
| return true; | ||
| } catch (error) { | ||
| process.chdir(originalDir); | ||
| if (error.status === 1 || error.status === 128) { |
There was a problem hiding this comment.
It would be helpful to comment on what these statuses represent or extract them into named constants.
| @@ -64,6 +47,7 @@ export const checkIsAncestor = async (commit: string): Promise<boolean> => { | |||
| return true; | |||
| } catch (error) { | |||
| process.chdir(originalDir); | |||
| // Error status 1 and 128 means that the commit is not an anecestor and the user must fetch. | |||
There was a problem hiding this comment.
I discussed this offline but would love to see a reference to this here. Just so its easy to differentiate between what 1 and 128 means.
EVG-20677
EVG-20868
Description
EVG-20677 was solved by not erroring on code 128 from
git merge-base --is-ancestor. All it means is that the commit has never been fetched by the developers git program.EVG-20868 contains an overview of the rest of the code changes.
I made sure to fail the script only if
types.tsis verified as stale. I pass the script for all other errors for the sake of developer productivity and also because this validation overlaps withcheck_codegen.I used
git mvfordiff-local-schema-with-remote->check-schema-and-codegenbut GitHub didn't do a good job at lining up the diff 😬 .