Skip to content
This repository was archived by the owner on Aug 20, 2025. It is now read-only.

Conversation

@tirumerla
Copy link
Contributor

@tirumerla tirumerla commented May 11, 2023

  • Fix formatting
  • Add pre-commits
  • Use Nx for faster builds, lints & tests
  • Add release drafter
  • Add dependabot config
  • Create automated rc & stable releases.

Runs CI/CD

  • Contains two workflows
    • Every push to master --> Updates RC in all the files --> creates & push signed commits using a svc account --> creates a rc tag --> creates a pre-release draft.
    • Manual trigger of stable workflow --> creates a stable tag --> creates a latest release --> published to @openzeppelin/...
    • There is a release workflow that can be used to trigger a release from any tag manually.

When i configure protected tag for some reason pushing tags fail not sure why.

Once this is merged all the packages will be start publishing to @openzeppelin so we need to update documentation for existing packages to let users know that we are using this going forward or migrate existing packages from defender-*-client to @openzeppelin/defender-*-client


FOLLOW UP

  • Change all examples to point to @openzeppelin/defender-*-client once the beta version is published.

'Accept': 'application/json, text/plain, */*',
'X-Api-Key': '4Rfp2GEHDjgesA6MdseUM1n8B8kT9hgs',
Authorization: 'Bearer WRONG',
'Authorization': 'Bearer WRONG',

Check failure

Code scanning / CodeQL

Hard-coded credentials

The hard-coded value "Bearer WRONG" is used as [authorization header](1).
@zeljkoX
Copy link
Contributor

zeljkoX commented May 11, 2023

LGTM

Just tested master(lerna + Nx) with this branch and speed improvements are visible

Screenshot 2023-05-11 at 13 17 38 Screenshot 2023-05-11 at 13 17 57

@socket-security
Copy link

socket-security bot commented May 16, 2023

New dependency changes detected. Learn more about Socket for GitHub ↗︎


👍 No new dependency issues detected in pull request

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] bar@* or ignore all packages with @SocketSecurity ignore-all

Pull request alert summary
Issue Status
Critical CVE ✅ 0 issues
CVE ✅ 0 issues
Mild CVE ✅ 0 issues
Install scripts ✅ 0 issues
Native code ✅ 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
GitHub dependency ✅ 0 issues
Potential typo squat ✅ 0 issues
Known Malware ✅ 0 issues
Telemetry ✅ 0 issues
Protestware/Troll package ✅ 0 issues
AI detected security risk ✅ 0 issues
AI warning ✅ 0 issues

📊 Modified Dependency Overview:

➕ Added Package Capability Access +/- Transitive Count Publisher
[email protected] eval, filesystem, shell, environment +3 altan-nrwl
[email protected] environment +11 nrwl-jason
[email protected] filesystem, environment +188 jameshenry

🚮 Removed packages: @ethersproject/[email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]

@tirumerla tirumerla marked this pull request as ready for review May 19, 2023 08:00
Copy link
Contributor

@collins-w collins-w left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks.

Copy link
Contributor

@zeljkoX zeljkoX left a comment

Choose a reason for hiding this comment

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

LGTM

Small comment about package versions.

},
"dependencies": {
"defender-sentinel-client": "1.44.0",
"@openzeppelin/defender-sentinel-client": "1.44.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure should we change this as we don't have yet this package published. This would cause examples not to work.

Same applies to other examples.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I want to check if we can migrate the existing packages already from defender-*-client to @openzeppelin/ i'm not sure if it's possible

Copy link
Contributor

@mok0230 mok0230 left a comment

Choose a reason for hiding this comment

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

Thanks for doing all this! Just a couple items to take a look at


- name: Publish
run: |
yarn publish
Copy link
Contributor

Choose a reason for hiding this comment

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

For a release candidate, will this use the next or beta flag (rather than latest)?

If it does use latest, it may cause npm install to cause users to upgrade to a beta version:

package.json

"openzeppelin/defender-admin-client": "^1.44.0",

may use 1.45.0-rc.1 if that were tagged latest. It's possible the -beta overrides this - not sure - but it would be worth checking.

Copy link
Contributor Author

@tirumerla tirumerla May 22, 2023

Choose a reason for hiding this comment

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

All the publishes ( from github actions) uses latest since we are not explicitly specifying --dist-tag with lerna publish

Copy link
Contributor

@mok0230 mok0230 left a comment

Choose a reason for hiding this comment

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

Thanks for doing all this! Just a couple items to take a look at

@tirumerla
Copy link
Contributor Author

Thanks for doing all this! Just a couple items to take a look at

Thanks for reviewing the PR. 🙏

Copy link
Contributor

@zeljkoX zeljkoX left a comment

Choose a reason for hiding this comment

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

Thanks for hard work!

@tirumerla
Copy link
Contributor Author

Thanks for hard work!

Thanks for reviewing the PR. 🙏🏽

@tirumerla tirumerla merged commit 86a1db0 into master May 23, 2023
@tirumerla tirumerla deleted the improve-ci-cd branch May 23, 2023 23:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants