Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add missing awaits #379

Merged
merged 4 commits into from
Nov 3, 2020
Merged

Add missing awaits #379

merged 4 commits into from
Nov 3, 2020

Commits on Oct 31, 2020

  1. auth-helper: properly await replacement of the token value in the config 

    After writing the `.extraheader` config, we manually replace the token
with the actual value. This is done in an `async` function, but we were
not `await`ing the result.

In our tests, this commit fixes a flakiness we observed where
`remote.origin.url` sometimes (very rarely, actually) is not set for
submodules. Our interpretation is that the configs are in the process of
being rewritten with the correct token value _while_ another `git
config` that wants to set the `insteadOf` value is reading the config,
which is currently empty.

A more idiomatic way to fix this in Typescript would use
`Promise.all()`, like this:

      await Promise.all(
        configPaths.map(async configPath => {
          core.debug(`Replacing token placeholder in '${configPath}'`)
          await this.replaceTokenPlaceholder(configPath)
        })
      )

However, during review of actions#379
it was decided to keep the `for` loop in the interest of simplicity.

Reported by Ian Lynagh.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
    @dscho
    dscho committed Oct 31, 2020
    Configuration menu
    Copy the full SHA
    8812d4a View commit details
    Browse the repository at this point in the history

  2. downloadRepository(): await the result of recursive deletions 

    Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
    @dscho
    dscho committed Oct 31, 2020
    Configuration menu
    Copy the full SHA
    db75315 View commit details
    Browse the repository at this point in the history

  3. Ask ESLint to report floating Promises 

    This rule is quite helpful in avoiding hard-to-debug missing `await`s.

Note: there are two locations in `src/main.ts` that trigger warnings:
the `run()` and the `cleanup()` function are called without `await` and
without any `.catch()` clause.

In the initial version of actions#379,
this was addressed by adding `.catch()` clauses. However, it was
determined that this is boilerplate code that will need to be fixed in a
broader way.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
    @dscho
    dscho committed Oct 31, 2020
    Configuration menu
    Copy the full SHA
    e5da2ad View commit details
    Browse the repository at this point in the history

  4. Rebuild 

    This trick was brought to you by `npm ci && npm run build`. Needed to
get the PR build to pass.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
    @dscho
    dscho committed Oct 31, 2020
    Configuration menu
    Copy the full SHA
    1607ea3 View commit details
    Browse the repository at this point in the history