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

chore: Add lockfile to integration-test project #570

Merged
merged 5 commits into from
May 19, 2021
Merged

chore: Add lockfile to integration-test project #570

merged 5 commits into from
May 19, 2021

Commits on May 19, 2021

  1. Revert "chore: Remove @guardian/eslint-config-typescript" 

    This reverts commit 8a3d86a.
    @akash1810
    akash1810 committed May 19, 2021
    Configuration menu
    Copy the full SHA
    5a96a58 View commit details
    Browse the repository at this point in the history

  2. Revert "chore: Add a note to the README for lint change" 

    This reverts commit cad2bf9.
    @akash1810
    akash1810 committed May 19, 2021
    Configuration menu
    Copy the full SHA
    b6ecf49 View commit details
    Browse the repository at this point in the history

  3. Revert "chore: Remove linting from integration-test project" 

    This reverts commit 9da786b.
    @akash1810
    akash1810 committed May 19, 2021
    Configuration menu
    Copy the full SHA
    d8fb59c View commit details
    Browse the repository at this point in the history

  4. chore: Fix import order linting error 

    I think this is failing because we do not observe a lockfile in the [integration-test project](https://github.com/guardian/cdk/blob/9b9a91bc03f507ba4d688024b4fc8059a617f9d0/integration-test/script/ci#L5-L8).
As `eslint-plugin-import`'s version includes `^`, we cannot guarantee what version gets used in CI.

We hadn't seen it locally as we don't do a fresh install. If you delete integration-test/node_modules and lint the integration-test project, the error can be seen.

Options:
  - Use pinned versions in the integration-test project (I'm not sure if dependabot works with pinned dependency versions)
  - Do not lint the integration-test project
  - Use a lockfile and install @guardian/cdk into the integration-test project with `--no-package-lock` (not sure if this is possible as @guardian/cdk will remain in `package.json`)

See:
  - import-js/eslint-plugin-import#2021
  - https://docs.npmjs.com/cli/v6/commands/npm-install#:~:text=--no-package-lock
    @akash1810
    akash1810 committed May 19, 2021
    Configuration menu
    Copy the full SHA
    cec95e6 View commit details
    Browse the repository at this point in the history

  5. chore: Add lockfile to integration-test project 

    We previously had issues with lockfiles in CI for the `integration-test` project.
This caused a few issues and led to the changes in #569.

This change reverts those commits and adds a lockfile.
That is, I'm not sure what the previous issues were, but they don't seem to
exist anymore and we can use lockfiles in CI and gain deterministic builds.
    @akash1810
    akash1810 committed May 19, 2021
    Configuration menu
    Copy the full SHA
    05b74f3 View commit details
    Browse the repository at this point in the history