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

Eslint default and pr template #871

Merged
merged 6 commits into from
Oct 17, 2023
Merged

Conversation

dr-bizz
Copy link
Contributor

@dr-bizz dr-bizz commented Oct 11, 2023

Description

Updating code base to use current ESLint & Prettier rules. This meant a lot of code needed to change, which lint:fix and prettier:write commands did most of the work. I fixed all the errors which couldn't be fixed automatically.
Also added PR template.

Changes

  • Updated esLint config with our standard rules
  • Updated Prettier with our standard rules
  • Updated package.json with packages needed for our Eslint standard rules.
  • Fix commands fixed most issues with code.
  • Afterwards, I manually combed through the files and fixed any lint errors that were left. mostly undefined and defined and not used.
  • Added a PR template for future PRs.

Side note: Build will fail until PR #870 is deployed.

@gitguardian
Copy link

gitguardian bot commented Oct 11, 2023

⚠️ GitGuardian has uncovered 2 secrets following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secrets in your pull request
GitGuardian id Secret Commit Filename
8455275 Rollbar API Access Token db16220 app/assets/javascripts/angular/campusContactsApp.constants.js View secret
8455275 Rollbar API Access Token db16220 app/assets/javascripts/angular/campusContactsApp.constants.js View secret
🛠 Guidelines to remediate hardcoded secrets
  1. Understand the implications of revoking this secret by investigating where it is used in your code.
  2. Replace and store your secrets safely. Learn here the best practices.
  3. Revoke and rotate these secrets.
  4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

Our GitHub checks need improvements? Share your feedbacks!

@wrandall22
Copy link
Contributor

From review it looks fine, but was waiting for #870 to put one here officially.

@dr-bizz dr-bizz force-pushed the eslint-default-and-pr-template branch from 6d5af90 to 7cd5582 Compare October 11, 2023 18:10
@wrandall22
Copy link
Contributor

What needs to happen for the Amplify preview to work?

@dr-bizz
Copy link
Contributor Author

dr-bizz commented Oct 12, 2023

They are working as I can see the preview in AWS. Yesterday I had to reconnect Amplify to this Repo, so that might have been the issue.

@dr-bizz
Copy link
Contributor Author

dr-bizz commented Oct 12, 2023

@wrandall22
Copy link
Contributor

wrandall22 commented Oct 12, 2023

The one I see in the console failed:

Screen Shot 2023-10-12 at 1 53 00 PM

2023-10-12T17:48:58.729Z [WARNING]: error eslint-plugin-n@16.2.0: The engine "node" is incompatible with this module. Expected version ">=16.0.0". Got "14.21.3"
2023-10-12T17:48:58.735Z [WARNING]: error Found incompatible module.
2023-10-12T17:48:58.736Z [INFO]: info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.
2023-10-12T17:48:58.752Z [ERROR]: !!! Build failed

@aws-amplify-us-east-1
Copy link

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-871.dyw2g0jg8t9yr.amplifyapp.com

@dr-bizz
Copy link
Contributor Author

dr-bizz commented Oct 12, 2023

I just updated the build image which is now on node 16. Now it's working.

I'm also testing the staging site with the node 16 build image.

@dr-bizz dr-bizz force-pushed the eslint-default-and-pr-template branch from dbc5ed7 to 17488e5 Compare October 16, 2023 14:16
@dr-bizz dr-bizz merged commit 1ccc2c9 into master Oct 17, 2023
6 checks passed
@dr-bizz dr-bizz deleted the eslint-default-and-pr-template branch October 17, 2023 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants