feat(KEY_CODES): warn about KEY_CODES removal #210
Conversation
| configs: { | ||
| recommended: { | ||
| plugins: ["pf-codemods"], | ||
| plugins: ["@patternfly/pf-codemods"], |
There was a problem hiding this comment.
This fixes an error introduced when we scoped the packages under @patternfly
| const severity = rule.includes('warn') ? 'warn' : 'error'; | ||
| acc[`@patternfly/pf-codemods/${rule}`] = severity; |
There was a problem hiding this comment.
This allows us to set rules as warnings on the eslint-plugin-pf-codemods side of things.
Alternatively we could set it in the eslintrc file in pf-codemods, not sure which approach would be better and fine with either, but I lean slightly towards this so that it's a warning regardless of if the pf-codemods package is being used or the eslint-plugin-pf-codemods package.
There was a problem hiding this comment.
I've since removed this as it seems some more discussion probably needs to happen around how we'd want this implemented, or if we even actually want it implemented.
| const fspath = require('path'); | ||
| const { CLIEngine } = require('eslint/lib/cli-engine'); | ||
| const { configs } = require('eslint-plugin-pf-codemods'); | ||
| const { configs } = require('@patternfly/eslint-plugin-pf-codemods'); |
There was a problem hiding this comment.
Another fix for the move to the @patternfly scope.
| // Don't show warnings | ||
| results.forEach(result => result.messages = result.messages.filter(message => message.severity === 2)); | ||
|
|
There was a problem hiding this comment.
This setting was making it so that warnings never surfaced before, it has to be removed so that we can issue them.
There was a problem hiding this comment.
can you think of a reason this was here in the first place?
There was a problem hiding this comment.
Not off the top of my head, and it was just added in this commit that changed things all over the repo 1ab03a0#diff-06072659dcb0ba81f72d49d18cc77c030ba13d5edacde533742c0dd8ac81730f
|
|
||
| // https://github.com/patternfly/patternfly-react/pull/8174 | ||
| module.exports = { | ||
| meta: { fixable: "code" }, |
There was a problem hiding this comment.
should this be here if we aren't doing a code fix? also, did you mean this rule to add a warning vs error?
There was a problem hiding this comment.
Hmmm, probably not? I'll try removing and see what happens.
And yes, the intent was for it to be a warning. If we want it to be an error though I'm fine with that.
| "test:single": "pf-codemods --no-cache test/test.tsx", | ||
| "test:koku": "pf-codemods --no-cache test/koku-ui", | ||
| "test:console": "pf-codemods --no-cache test/console/frontend", | ||
| "test:integreatly": "pf-codemods --no-cache test/tutorial-web-app" |
There was a problem hiding this comment.
This makes it so that tests don't create a cache file, the cache file was causing issues with testing and needing to be manually deleted after each run.
| .option('--exclude <rules>', 'Run recommended rules EXCLUDING this comma-seperated list') | ||
| .option('--fix', 'Whether to run fixer') | ||
| .option('--format <format>', 'What eslint report format to use', 'stylish') | ||
| .option('--no-cache', 'Disables eslint caching') |
There was a problem hiding this comment.
This + line 76 add a CLI option that allows us to selectively use eslint caching. Caching defaults to true so that consumers using it still get the benefits of the caching.
You can read more about what I'm actually doing here if you're curious: https://www.npmjs.com/package/commander#options
Closes #134
Also address some bugs and enables us to have rules as warnings rather than just errors.