-
Notifications
You must be signed in to change notification settings - Fork 26
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
base: master
Are you sure you want to change the base?
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.
… for e2e test local/github
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.
"-g", | ||
"daemon off;" | ||
] | ||
dockerfile: Dockerfile |
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.
Are you sure that we do not need to wait for the service to come on line?
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
depends_on: | ||
mongodb: | ||
condition: service_healthy | ||
|
||
backend: | ||
image: ghcr.io/scicatproject/backend-next:latest | ||
command: sh -c "node dist/main" |
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.
I think the command is part of the image already and could be removed
@@ -1,14 +1,16 @@ | |||
version: "3.4" | |||
services: | |||
reverse-proxy: | |||
image: traefik:2.5 | |||
image: traefik:2.10 |
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.
I am not sure I understand what we need traefik for
"-g", | ||
"daemon off;" | ||
] | ||
dockerfile: Dockerfile |
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.
I think this line could be removed as Dockerfile
should be the defautl
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: