Skip to content

Add Javascript files to PHPCS scan#5

Merged
claudiulodro merged 3 commits into
masterfrom
add/js-to-phpcs-sniffs
May 6, 2019
Merged

Add Javascript files to PHPCS scan#5
claudiulodro merged 3 commits into
masterfrom
add/js-to-phpcs-sniffs

Conversation

@philipjohn
Copy link
Copy Markdown
Contributor

All Submissions:

Changes proposed in this Pull Request:

To make full use of the VIP Go standard we should include JS, as there are some handy rules in the Go standard that check for potential XSS vulnerabilties.

To make full use of the VIP Go standard we should include JS, as there are some handy rules in the Go standard that check for potential XSS vulnerabilties.
Copy link
Copy Markdown
Contributor

@claudiulodro claudiulodro left a comment

Choose a reason for hiding this comment

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

When I add a random JS file and try committing I get the following error when running the precommit hooks:

Installing JSHint

Failed to install jshint (try installing as a local package via 'npm install --save-dev jshint'), so skipping jshint

Does the JS linting work in your testing? It seems like we need a package.json.

@claudiulodro claudiulodro added [Status] Needs Changes or Feedback The issue or pull request needs action from the original creator [Status] Needs Review The issue or pull request needs to be reviewed and removed [Status] Needs Changes or Feedback The issue or pull request needs action from the original creator [Status] Needs Review The issue or pull request needs to be reviewed labels Mar 6, 2019
@claudiulodro
Copy link
Copy Markdown
Contributor

I've pushed an update to exclude the dist folder from the rules. Everything else seems to work well. The JSHint error I mention above is unrelated to this. Between this and Prettier we should have more than enough JS linting.

@claudiulodro claudiulodro removed the [Status] Needs Changes or Feedback The issue or pull request needs action from the original creator label May 6, 2019
@claudiulodro claudiulodro added the [Status] Approved The pull request has been reviewed and is ready to merge label May 6, 2019
@claudiulodro claudiulodro merged commit e845a00 into master May 6, 2019
@claudiulodro claudiulodro deleted the add/js-to-phpcs-sniffs branch May 6, 2019 18:10
@dkoo dkoo mentioned this pull request Jan 26, 2021
6 tasks
kmwilkerson added a commit that referenced this pull request May 15, 2026
- Add isEligible guard on Reset: excluded for woocommerce-source emails
  and emails without a registry_slug (finding #1)
- Capture previous status before optimistic update so rollback doesn't
  assume binary publish/draft (finding #2)
- Drop view_category from enrichment response — dead data not consumed
  by the frontend (finding #3)
- Add @todo NPPD-1532 comment on reset endpoint coupling (finding #4)
- Add NPPD-1531 comment on optimistic update / store consolidation (#5)
- Replace brittle PHPUnit count assertions with structural invariants:
  required keys, valid source/recipient, exclusive type keys, no
  duplicates, sort order (findings #6, #7, #8)
- Add optimistic-rollback failure test and reset eligibility test (#9, #11)
- Add 6th mockEmail fixture (WC draft) for cleaner activate test (#10)
- Coerce pluginsReady with Boolean() to handle undefined (#12)
- Use noticeText prop consistently on Notice (#13)
- Fix preview placeholder to use neutral gray tokens (#14)
- Trim EmailItem interface to consumed fields, add source (#15)
- Add source-of-truth comment on category_order strings (#16)
- Consolidate three duplicate TODO comments into one (#17)
- Add NPPD-1525 ticket reference on EmailPreview TODO (#18)
- Extract duplicated screen-reader h1 into PageHeading component (#19)
- Add text-overflow ellipsis on trigger description column (#20)
- Return localized string from recipient getValue for search (#21)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Status] Approved The pull request has been reviewed and is ready to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants