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.

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

339 eslint production build #340

Merged
merged 13 commits into from
Dec 18, 2023
Merged

339 eslint production build #340

merged 13 commits into from
Dec 18, 2023

Conversation

alishaevn
Copy link
Contributor

@alishaevn alishaevn commented Dec 15, 2023

Story

prevent eslint errors from failing a production build

Expected Behavior After Changes

  • eslint doesn't blow up the production build
  • eslint is run in it's own workflow
  • the cypress ci pipeline works, but is failing. (out of scope to fix it here, but documented in EPIC: fix cypress specs #341)

Screenshots / Video

ref: this comment

we still need to check eslint somewhere in this process though. so I'm
creating the build-test-lint workflow.

ref: https://nextjs.org/docs/pages/api-reference/next-config-js/eslint
this commit change means we no longer get the error:
  "Detected next.config.js, no exported configuration found.
  https://nextjs.org/docs/messages/empty-configuration"
Copy link

vercel bot commented Dec 15, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
webstore-staging ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 18, 2023 9:01pm

// Warning: This allows production builds to successfully complete even if
// your project has ESLint errors.
// ref: https://nextjs.org/docs/pages/api-reference/next-config-js/eslint
ignoreDuringBuilds: true,
Copy link
Contributor Author

@alishaevn alishaevn Dec 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I confirmed that this works by pushing this branch to production on vercel, while eslint still has a failure.
Screenshot 2023-12-15 at 6 35 25 PM
image

@alishaevn alishaevn linked an issue Dec 16, 2023 that may be closed by this pull request
2 tasks
Comment on lines +38 to +39
// TODO(alishaevn): add these cookies to pages/legal-notices/cookie-policy.js under
// "Non-essential cookies" with a description and expiration time frame
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

split into 2 lines to appease lint rules

the existing line failed with a "Error: Cannot find module
'/home/node/app/bash'" error. Locally and in CI/CD. I found a SO post
and tried the suggestion, which worked locally. I'm hoping it will work
in CI/CD too.

ref: https://stackoverflow.com/a/49647309/8079848
trying to get around the error:
  > Cypress could not verify that this server is running:
  > http://localhost:3000
  > We are verifying this server because it has been configured as your
  > baseUrl.
Comment on lines +25 to +34
cypress:
runs-on: ubuntu-22.04
steps:
- name: Checkout
uses: actions/checkout@v4
- name: Cypress e2e
uses: cypress-io/github-action@v6
with:
start: yarn start
wait-on: 'http://localhost:3000'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines +35 to +39
eslint:
needs: build
uses: scientist-softserv/actions/.github/workflows/[email protected]
with:
lint_cmd: docker compose run web sh -c 'yarn && yarn lint'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ref: https://github.com/scientist-softserv/viva/blob/main/.github/workflows/build-test-lint.yaml#L36-L40

I had to edit the lint_cmd slightly because I kept getting an error about not being able to find "bash".

Comment on lines +1 to +24
name: 'Build Test Lint'
run-name: Build Test Lint of ${{ github.ref_name }} by @${{ github.actor }}
on:
push:
branches:
- main
pull_request:
branches:
- main
workflow_dispatch:
inputs:
debug_enabled:
type: boolean
description: 'Run the build with tmate debugging enabled (https://github.com/marketplace/actions/debugging-with-tmate)'
required: false
default: false

jobs:
build:
uses: scientist-softserv/actions/.github/workflows/[email protected]
secrets: inherit
with:
platforms: 'linux/amd64'
webTarget: web
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this already existed in "build.yml". I just moved it here.

Comment on lines +27 to +28
# remove the line below to enable the job
if: false
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using this conditional to skip the workflow instead of commenting out the file.

ref: https://stackoverflow.com/a/75905933/8079848

@alishaevn alishaevn merged commit 2609de2 into main Dec 18, 2023
6 of 7 checks passed
@alishaevn alishaevn deleted the 339-eslint-production-build branch December 18, 2023 22:14
This was linked to issues Jan 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

eslint fails production builds restore the test-suite github action create a lint github action
2 participants