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

chore: remove unused runcitadel sdk #1721

Merged
merged 4 commits into from
Nov 8, 2022
Merged

chore: remove unused runcitadel sdk #1721

merged 4 commits into from
Nov 8, 2022

Conversation

im-adithya
Copy link
Member

Describe the changes you have made in this PR

Removes the citadel npm package

Link this PR to an issue [optional]

Step 2: #1314

Type of change

(Remove other not matching type)

  • chore

Checklist

  • My code follows the style guidelines of this project and performed a self-review of my own code
  • New and existing tests pass locally with my changes
  • I checked if I need to make corresponding changes to the documentation (and made those changes if needed)

@im-adithya im-adithya requested a review from escapedcat November 7, 2022 12:47
@github-actions
Copy link

github-actions bot commented Nov 7, 2022

🚀 Thanks for the pull request!

Here are the current build files for testing:

Download and unzip the file for your browser. Refer to the readme for detailed install instructions.


This build is brought to you by: Adam Fiscor (who recently dropped 1337 sats):

The future is bright. We just have a lot of work to do.

Want to sponsor the next build? send some sats to ⚡️[email protected] (don't forget to provide your name)

Don't forget: keep earning sats!

jest.config.js Outdated
@@ -28,7 +28,7 @@ module.exports = {
],
"^.+\\.(t|j)sx?$": ["@swc/jest", swcConfig],
},
transformIgnorePatterns: ["node_modules/(?!(@runcitadel))/"],
transformIgnorePatterns: ["node_modules/(?!(dexie))/"],
Copy link
Collaborator

Choose a reason for hiding this comment

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

why was dexie added here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because Jest is failing: https://github.com/getAlby/lightning-browser-extension/pull/747/files#r842242790 [same reason but dexie ig]

Not exactly sure why.
@escapedcat knows: #1314 (comment)

Unit-test setup

Looks like some unit-tests fail after an update to 16/18.

This helps partly:

Copy link
Contributor

Choose a reason for hiding this comment

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

From jest:

    Jest encountered an unexpected token

    Jest failed to parse a file. This happens e.g. when your code or its dependencies use non-standard JavaScript syntax, or when Jest is not configured to support such syntax.

    Out of the box Jest supports Babel, which will be used to transform your files into valid JS based on your Babel configuration.

    By default "node_modules" folder is ignored by transformers.

    Here's what you can do:
     • If you are trying to use ECMAScript Modules, see https://jestjs.io/docs/ecmascript-modules for how to enable it.
     • If you are trying to use TypeScript, see https://jestjs.io/docs/getting-started#using-typescript
     • To have some of your "node_modules" files transformed, you can specify a custom "transformIgnorePatterns" in your config.
     • If you need a custom transformation specify a "transform" option in your config.
     • If you simply want to mock your non-JS modules (e.g. binary assets) you can stub them out with the "moduleNameMapper" config option.

    You'll find more details and examples of these config options in the docs:
    https://jestjs.io/docs/configuration
    For information about custom transformations, see:
    https://jestjs.io/docs/code-transformation

@socket-security
Copy link

Socket Security Pull Request Report

Dependency issues detected. If you merge this pull request, you will not be alerted to the instances of these issues again.

📜 Install scripts

Install scripts are run when the package is installed. The majority of malware in npm is hidden in install scripts.

Packages should not be running non-essential scripts during install and there are often solutions to problems people solve with install scripts that can be run at publish time instead.

Package Script field Location
@swc/[email protected] (upgraded) postinstall package.json via @swc/[email protected]
Pull request report summary
Issue Status
Install scripts ⚠️ 1 issue
Native code ✅ 0 issues
Bin script confusion ✅ 0 issues
Bin script shell injection ✅ 0 issues
Unresolved require ✅ 0 issues
Invalid package.json ✅ 0 issues
HTTP dependency ✅ 0 issues
Git dependency ✅ 0 issues
Potential typo squat ✅ 0 issues
Known Malware ✅ 0 issues
Telemetry ✅ 0 issues
Protestware/Troll package ✅ 0 issues
Bot Commands

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of package-name@version specifiers. e.g. @SocketSecurity ignore [email protected] [email protected]

⚠️ Please accept the latest app permissions to ensure bot commands work properly. Accept the new permissions here.

Powered by socket.dev

@escapedcat
Copy link
Contributor

Socket Security Pull Request Report

Not sure why these packages are named. They haven't been changed in the this PR. At least they do not shpw up as changed in the lock-file diff or am I missing something?

@bumi bumi merged commit 2370535 into master Nov 8, 2022
@bumi bumi deleted the task-remove-citadel branch November 8, 2022 22:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants