chore: Improve code quality #77
Conversation
| "n8n-workflow": "1.82.0" | ||
| }, | ||
| "dependencies": { | ||
| "axios": "^1.13.2" |
There was a problem hiding this comment.
We are not allowed to have dependencies
| this.helpers.httpRequestWithAuthentication.call(this, authenticationMethod, options), | ||
| ); | ||
| } catch (error) { | ||
| /** |
There was a problem hiding this comment.
We need to change the error handling below. We should use typeof as they requested
| "lint": "eslint nodes credentials package.json", | ||
| "lintfix": "eslint nodes credentials package.json --fix", | ||
| "prepublishOnly": "pnpm build && pnpm lint -c .eslintrc.prepublish.js nodes credentials package.json", | ||
| "prepublishOnly": "npm build && npm lint -c .eslintrc.prepublish.js nodes credentials package.json", |
There was a problem hiding this comment.
Sorry forgot to mention in the ticket description, we need to remove prepublishOnly as well
| - name: Commit version update | ||
| run: | | ||
| git add package.json pnpm-lock.yaml | ||
| git add package.json npm-lock.yaml |
|
|
||
| - name: Install dependencies | ||
| run: pnpm install --frozen-lockfile | ||
| run: npm install --frozen-lockfile |
There was a problem hiding this comment.
npm doesn't have --frozen-lockfile command I think we can change it for npm ci
|
|
||
| - name: Install dependencies | ||
| run: pnpm install --frozen-lockfile | ||
| run: npm install --frozen-lockfile |
There was a problem hiding this comment.
Again no --frozen-lockfile I think it should be npm ci
|
|
||
| - name: Format check | ||
| run: pnpm exec prettier --check nodes credentials | ||
| run: npx prettier --check nodes credentials |
There was a problem hiding this comment.
nit: I think we should add this command into the package.json like
"format:check": "prettier --check nodes credentials"
| 2. Version extraction (`X.Y.Z`) from the release tag. | ||
| 3. Build and test processes. | ||
| 4. Update `package.json` and `pnpm-lock.yaml` to version `X.Y.Z`. | ||
| 4. Update `package.json` and `npm-lock.yaml` to version `X.Y.Z`. |
|
Also I think it might be worth renaming the PR title to fix. I think there are a lot of fixes. What do you think? |
drobnikj
left a comment
drobnikj
left a comment
There was a problem hiding this comment.
Agree with @protoss70 notes, otherwise fine.
3 participants
Closes issue
httpRequestWithAuthenticationexec()function and when triggered the functionality is called fromn8n-coreso we would have to mock this logic somehow, and the tests would have ended up being just mock values from mock functions passed around, which is not worth it in my opinionMost of the changes in the PR are caused by the change of npm for pnpm. They have different files for dependencies so the old
pnpm-lock.yamlwas deleted the newpackage-lock.jsonfile