-
Notifications
You must be signed in to change notification settings - Fork 30
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: remove unused and outdated files #1624
refactor: remove unused and outdated files #1624
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @Junjiequan - I've reviewed your changes - here's some feedback:
Overall Comments:
- The removal of the ESS and MAXIV pipeline triggers in the deploy.yml workflow is a significant change. Please document the reasons for their removal and provide information on any replacement processes for these deployments.
- Consider reviewing the startup dependencies for all services in the docker-compose file. While you've added a health check for the backend service, ensure that all services start up in the correct order to prevent potential race conditions.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one question.
The rest looks fine to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as this PR seems to do some cleanup, I would suggest to have a look at this, at least for deploy.yml
, maybe as food for thoughts for another PR (or at leat an issue). In this way we could reuse the deploy.yml workflow in all repos and import from a central place
… for e2e test local/github
88f57ed
to
741e582
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
* refactor: remove unused and outdated files * fix failing e2e test * fix failing e2e test * fix for cypress config type error & keep only one cypress config file for e2e test local/github * fix jesmine asserstion fake error * keep only one cypress.config.ts * fix failing e2e * revert scicat-sdk-ts import * fix for commented issues
Description
This PR aims to cleanup outdated CI/E2E configurations.
Main fixes:
Motivation
The CI and testing setup has accumulated several unused and unmaintained configuration files, leading to increased difficulty in maintaining and debugging the system
Fixes:
Please provide a list of the fixes implemented in this PR
Changes:
Please provide a list of the changes implemented by this PR
Tests included
Documentation
official documentation info
If you have updated the official documentation, please provide PR # and URL of the pages where the updates are included
Backend version
Summary by Sourcery
Refactor the project by removing unused and outdated files, updating docker-compose configurations, and optimizing CI workflows. Update service images and configurations to improve compatibility and performance. Remove unnecessary pipeline triggers and enhance test workflow settings.
Enhancements:
CI: