Simplified initialSetup() function

[jamessimone]

• swapped let for const when appropriate
• simplified initialSetup() function

[jamessimone]

@mitchspano just wanted to get a little PR going here, but I have a much bigger question - any reason for not using TypeScript for this repo? It would make refactoring some of these functions a lot easier (especially places where the sync version of fs functions are being used at present) and allow you to express things like the severity levels being returned from the scanner in a type-safe way

[jamessimone]

As an example of why I find TS to be the ultimate helper for these things - it found the (obvious) bug in the solution I just submitted:

I'll snap off another commit with that fix

[mitchspano]

Hey @jamessimone thank you so much for your contribution!

Regarding TypeScript, I would be definitely open to refactoring this to TypeScript - I only chose JavaScript because of my familiarity with the language and the readily available documentation.

If we can make this work with TypeScript it would provide the benefits you mentioned above and (perhaps selfishly) be an excellent learning opportunity for me to get familiar with a new language.

[jamessimone]

@mitchspano I included another example of how helpful TypeScript can be in the PR I tagged you in on my own fork. There is going to be (in my experience locally developing this application) a significant pain point in keeping node_modules as part of this project's source code, but we would also need to prove out that the tools which bundle dependencies with a TypeScript action actually include the sfdx-cli dependency (since that is being invoked via a child process, which probably precludes the ability for tree shaking to find the dependency properly).

Regardless - I think there's a lot of potential to do additional cleanup even if the project remains entirely javascript-based. Here's an example of what I'm talking about (which I only have locally):

async function main() {
  const { inputs, pullRequest, scannerCliArgs } = initialSetup();
  validatePullRequestContext(pullRequest);
  const filePathToChangedLines = getDiffInPullRequest(pullRequest);
  await recursivelyMoveFilesToTempFolder(filePathToChangedLines);
  const diffFindings = performStaticCodeAnalysisOnFilesInDiff(scannerCliArgs);
  const existingComments = await getExistingComments(pullRequest);
  const filePathToComments = filterFindingsToDiffScope(
    diffFindings,
    filePathToChangedLines,
    inputs,
    pullRequest
  );
  writeComments(
    existingComments,
    filePathToComments,
    pullRequest,
    hasHaltingError // this is still a "global" variable, but now it's the only one
  );
}

Changes like these are meant to remove variables from global state, creating a more idiomatic program control flow. I'm happy to re-create these changes in the pure javascript version here as well.

[mitchspano]

Yes, I would like to remove node_modules, however when writing a Javascript GitHub action they actually instruct you to include node_modules:

There is an alternative which involves using vercel to compile all the contents of node_modules into one file, but when I tried following the provided instructions, I was unable to get it to work, so I fell-back on ye olden node_modules folder.

I have created this issue to track the work of removal of node_modules in favor of Vercel.