Conversation
|
Thank you for your contribution! 🙌 I will be reviewing your code ASAP. 🔍💻 |
zleypner
left a comment
zleypner
left a comment
There was a problem hiding this comment.
Hello @od-hunter,
After reviewing your Next.js CI/CD implementation, I would recommend some improvements to enhance its robustness and security. You should upgrade to Node 20 and the latest GitHub Actions (v4) , implement proper environment separation between development, staging, and production, and strengthen security with dependency scanning and container image verification. The pipeline would also benefit from improved caching for faster builds, health checks for containers, and better environment variable management.
Additionally, your testing strategy should be expanded to include E2E, performance, and accessibility tests to ensure comprehensive coverage. Consider implementing canary deployments for safer releases, adding proper monitoring for the pipeline and application, and improving the local development experience with mock services. These changes will significantly enhance pipeline reliability, application security, and developer productivity while reducing deployment risks in your production environment. Please let me know if you have any question.
There was a problem hiding this comment.
Hello @od-hunter
🚧 Missing Improvements
Upgrade versions Use Node.js 20
Use latest GitHub Actions: checkout@v4, setup-node@v4
Environment handling
Separate dev, staging, and prod environments
Use conditional logic or matrix builds
Security
Add CodeQL analysis
Include dependency scanning and container verification
Caching
Implement actions/cache for .next/cache, node_modules, and Docker layers
Testing
Expand coverage with E2E, performance, and accessibility tests
Deployment & monitoring
Use canary/preview deployments for PRs
Add monitoring/logging/alerting after deploy
Also, I noticed these Stellar SDK-related dependencies were removed from package-lock.json:
@stellar/js-xdr
@stellar/stellar-base
@stellar/stellar-sdk
If this was not intentional, please make sure they’re reinstated — their removal could impact any feature that relies on them.
Once it has been updated please share a video with the Evidence. Thank you!
|
@od-hunter |
|
@zleypner Yes, I'll push an update soon |
feat: implememnt `Light/Dark` with `next/theme` over custom
…rontend-SafeTrust into CI/CD-Implementation
@zleypner I checked and I didn't touch the But I could also just revert the package-lock.json file. |
|
Hello @od-hunter , I will double check in some hours, what is causing this. |
|
Hello @od-hunter The error persist. Please fix |
|
Closing this PR due to inactivity for over one week. No updates or responses to feedback have been received since last March 31st. Thank you for your contribution! |
@zleypner You said you wanted to check what was causing that cos ideally the import was fine from my end. |
|
@od-hunter , Thank you for the updates. Testing |
|
Hi @zleypner please review |
|
Hello @od-hunter, there are some linter issues, for this PR we don't need any linter code or dependency Could you explain me why are you using Python in Dockerfile please? I would love to understand We're still checking the issue/pr |
I used python because of the usb package which was failing the docker with this error. npm error path /app/node_modules/usb The usb package is a dependency of @creit.tech/stellar-wallets-kit via @trezor/connect-plugin-stellar and @trezor/connect-web, causing the Docker build to fail without Python. |
4 participants
Pull Request for SafeTrust - Close Issue #185
❗ Pull Request Information
CI/CD Pipeline Implementation on Frontend
Add here some information
🌀 Summary of Changes
🛠 Testing
Evidence Before Solution
Evidence After Solution
📂 Related Issue
This pull request will close # [185] upon merging.
Make sure to follow the Git Guidelines for Atomic Commits and read Contributing Guide
The Pull request needs to have the format mentioned below in the Git Guideline
🎉 Thank you for reviewing this PR! 🎉