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

refactor: Delete unnecessary interface re-exports from core, and delete unused code in nodes-base (no-changelog) #6631

Merged
merged 2 commits into from
Jul 12, 2023

Conversation

netroy
Copy link
Member

@netroy netroy commented Jul 10, 2023

No description provided.

@github-actions
Copy link
Contributor

Great PR! Please pay attention to the following items before merging:

Files matching packages/**:

  • If fixing bug, added test to cover scenario.
  • If addressing forum or Github issue, added link to description.

Files matching packages/**/*.ts:

  • Added unit tests to cover new or updated functionality.

Files matching packages/nodes-base/nodes/**:

  • Added workflow tests for nodes if possible.

Make sure to check off this list before asking for review.

@@ -1319,22 +1319,22 @@ export class Gitlab implements INodeType {
const items = this.getInputData();
const returnData: INodeExecutionData[] = [];

let _credentials;
Copy link
Member Author

Choose a reason for hiding this comment

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

since _credentials used anywhere, is this dead-code copy-pasted from elsewhere?

Copy link
Member

Choose a reason for hiding this comment

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

Looks like it might be taken from a credential test

@netroy netroy changed the title refactor: delete unnecessary interface exports from core, and delete unused code in nodes-base (no-changelog) refactor: Delete unnecessary interface re-exports from core, and delete unused code in nodes-base (no-changelog) Jul 10, 2023
@@ -373,7 +373,7 @@ export class CrateDb implements INodeType {
returning,
);
}
const _updateItems = await db.multi(pgp.helpers.concat(queries));
// const _updateItems = await db.multi(pgp.helpers.concat(queries));
Copy link
Member Author

Choose a reason for hiding this comment

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

is this unused code?

Copy link
Member

Choose a reason for hiding this comment

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

Looks like it, I don't have a cratedb instance running locally but I can't it being used anywhere else.

@n8n-assistant n8n-assistant bot added core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team node/improvement New feature or request labels Jul 10, 2023
@netroy netroy force-pushed the remove-reexported-interfaces-from-core branch from 41bc060 to 0496bc3 Compare July 11, 2023 16:48
@codecov
Copy link

codecov bot commented Jul 11, 2023

Codecov Report

Patch coverage: 72.91% and project coverage change: -0.07 ⚠️

Comparison is base (d6b2ae0) 28.93% compared to head (0496bc3) 28.87%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6631      +/-   ##
==========================================
- Coverage   28.93%   28.87%   -0.07%     
==========================================
  Files        3071     3071              
  Lines      188445   188434      -11     
  Branches    20893    20889       -4     
==========================================
- Hits        54532    54402     -130     
- Misses     133033   133152     +119     
  Partials      880      880              
Impacted Files Coverage Δ
...kages/nodes-base/nodes/Chargebee/Chargebee.node.ts 0.00% <ø> (ø)
packages/nodes-base/nodes/CrateDb/CrateDb.node.ts 0.00% <0.00%> (ø)
packages/nodes-base/nodes/Disqus/Disqus.node.ts 0.00% <ø> (ø)
packages/nodes-base/nodes/Git/Git.node.ts 0.00% <ø> (ø)
packages/nodes-base/nodes/Gitlab/Gitlab.node.ts 0.00% <ø> (ø)
.../nodes/Google/BigQuery/v1/GoogleBigQueryV1.node.ts 10.41% <ø> (ø)
.../nodes/Google/BigQuery/v2/GoogleBigQueryV2.node.ts 100.00% <ø> (ø)
...e/BigQuery/v2/actions/database/insert.operation.ts 54.79% <ø> (ø)
...es-base/nodes/Google/BigQuery/v2/actions/router.ts 83.33% <ø> (ø)
...s-base/nodes/Google/BigQuery/v2/transport/index.ts 13.79% <ø> (ø)
... and 82 more

... and 5 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@netroy netroy requested a review from Joffcom July 11, 2023 17:37
Copy link
Member

@Joffcom Joffcom left a comment

Choose a reason for hiding this comment

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

Done some testing, All looks good to me 👍🏻

@cypress
Copy link

cypress bot commented Jul 12, 2023

Passing run #1427 ↗︎

0 219 0 0 Flakiness 0

Details:

🌳 remove-reexported-interfaces-from-core 🖥️ browsers:node18.12.0-chrome107 🤖 ...
Project: n8n Commit: 0496bc3cf0
Status: Passed Duration: 07:11 💡
Started: Jul 12, 2023 9:04 AM Ended: Jul 12, 2023 9:11 AM

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@github-actions
Copy link
Contributor

✅ All Cypress E2E specs passed

@netroy netroy merged commit e57e85e into master Jul 12, 2023
@netroy netroy deleted the remove-reexported-interfaces-from-core branch July 12, 2023 09:15
MiloradFilipovic added a commit that referenced this pull request Jul 12, 2023
* master:
  ci!: Drop support for Node.js 16. Add initial support for Node.js 20 (#6649)
  feat(API): Implement users account quota guards  (#6434)
  feat(OpenAI Node): Update max token limit to support newer model limits (#6644)
  ci: Run checklist job as a separate workflow (no-changelog) (#6645)
  fix(HTTP Request Node): Cleanup circular references in response (#6590)
  fix(Postgres Node): Upsert does not fetch columns when schema other then public (#6643)
  fix(core): Update docker compose setup for V1 images (#6642)
  refactor: Delete unnecessary interface re-exports from core, and delete unused code in nodes-base (no-changelog) (#6631)
  fix(core): Reduce memory consumption on `BinaryDataManager.init` (#6633)
  fix(editor): Implement simple view for templates experiment (no-changelog) (#6637)

# Conflicts:
#	packages/editor-ui/src/stores/settings.store.ts
MiloradFilipovic added a commit that referenced this pull request Jul 13, 2023
* ADO-814-trial-banner:
  ✔️ Updated callout component snapshots
  👌 Moving DOM logic from the store
  👌 Addressing more points from code review
  👌 Refactoring trial banner component and ui store
  👌 Updating permanent banner dismiss endpoint and back-end logic
  ⚡ Updating the migrations syntax
  ci!: Drop support for Node.js 16. Add initial support for Node.js 20 (#6649)
  feat(API): Implement users account quota guards  (#6434)
  feat(OpenAI Node): Update max token limit to support newer model limits (#6644)
  ci: Run checklist job as a separate workflow (no-changelog) (#6645)
  fix(HTTP Request Node): Cleanup circular references in response (#6590)
  fix(Postgres Node): Upsert does not fetch columns when schema other then public (#6643)
  fix(core): Update docker compose setup for V1 images (#6642)
  refactor: Delete unnecessary interface re-exports from core, and delete unused code in nodes-base (no-changelog) (#6631)
  fix(core): Reduce memory consumption on `BinaryDataManager.init` (#6633)
  ⚡ Updating upgrade plan button click
  👌 Addressing design review feedback
  ⚡ Using query parameters in migrations
  fix(editor): Implement simple view for templates experiment (no-changelog) (#6637)
  ⚡ Updating migration files
@janober
Copy link
Member

janober commented Jul 26, 2023

Got released with n8n@1.1.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team node/improvement New feature or request Released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants