Migrate repoPolicyCheck to OCLIF #11509
Conversation
RishhiB
requested review from
tylerbutler,
curtisman,
scottn12 and
sonalivdeshpande
Co-authored-by: Tyler Butler <tyler@tylerbutler.com>
Co-authored-by: Tyler Butler <tyler@tylerbutler.com>
Co-authored-by: Tyler Butler <tyler@tylerbutler.com>
Co-authored-by: Tyler Butler <tyler@tylerbutler.com>
Co-authored-by: Tyler Butler <tyler@tylerbutler.com>
…luidFramework-1 into migrateRepoPolicyCheck
| this.error("ERROR: No exclusions file provided."); | ||
| } | ||
|
|
||
| const exclusionsFile = await readJsonAsync(this.processedFlags.exclusions); |
There was a problem hiding this comment.
Does readjson return a string? You might need to parse it into an object first.
There was a problem hiding this comment.
readJsonAsync seems to return a JSON object, this may throw an error. I think I should wrap it in a try catch.
code for function;
export async function readJsonAsync(filename: string) {
const content = await readFileAsync(filename, "utf-8");
return JSON.parse(content);
}
build-tools/packages/build-tools/src/repoPolicyCheck/handlers/index.ts
Outdated
Show resolved
Hide resolved
…index.ts Co-authored-by: Tyler Butler <tyler@tylerbutler.com>
| export { handler as assertShortCodeHandler } from "./repoPolicyCheck/handlers/assertShortCode"; | ||
| export { handlers as copyrightFileHeaderHandlers } from "./repoPolicyCheck/handlers/copyrightFileHeader"; | ||
| export { handler as dockerfilePackageHandler } from "./repoPolicyCheck/handlers/dockerfilePackages"; | ||
| export { handler as fluidCaseHandler } from "./repoPolicyCheck/handlers/fluidCase"; | ||
| export { handlers as lockfilesHandlers } from "./repoPolicyCheck/handlers/lockfiles"; | ||
| export { handlers as npmPackageContentsHandlers } from "./repoPolicyCheck/handlers/npmPackages"; |
There was a problem hiding this comment.
You shouldn't need these exports anymore.
There was a problem hiding this comment.
Thanks for the catch!
| * Licensed under the MIT License. | ||
| */ | ||
|
|
||
| import { assertShortCodeHandler, copyrightFileHeaderHandlers, dockerfilePackageHandler, fluidCaseHandler, Handler, lockfilesHandlers, npmPackageContentsHandlers } from "../.."; |
There was a problem hiding this comment.
import these individually from the source files. e.g.
import {assertShortCodeHandler} from "./assertShortCode";What you're doing here is creating a new module, and the API for the module is defined by what's exported from this file (index.ts). The individual handlers are "private" to the module, so you import them here and then export them in the shape that you want for your "external/public" API for the module. When you make this change and remove the exports from the root index.ts, you'll effectively "hide" all the individual handlers from the outside world. Even code within the project but outside the module won't be able to access them directly without a lint error. All access will be through the array exported here.
There was a problem hiding this comment.
Sorry, I was in the process of doing that, please let me know if this is correct; 03ce23a
|
|
||
| type policyAction = "handle" | "resolve" | "final"; | ||
|
|
||
| export class CheckPolicy extends BaseCommand<typeof CheckPolicy.flags> { |
There was a problem hiding this comment.
Can you add some TSDoc comments to the class and functions? Imagine you have to do this again in 6 months -- what do you wish you'd known?
Co-authored-by: Tyler Butler <tyler@tylerbutler.com>
Co-authored-by: Tyler Butler <tyler@tylerbutler.com>
Co-authored-by: Tyler Butler <tyler@tylerbutler.com>
|
Thanks for all the help @tylerbutler and @sonalivdeshpande !! |
|
This commit is queued for merging with the |
AB#999