-
Notifications
You must be signed in to change notification settings - Fork 78
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
LF-2758 export server cicd #2483
Conversation
Some explanation of the script: the |
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.
Sorry it took so long to review but looks good 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.
I misunderstood the PR comments and thought that the nvm and git stash lines were an addition from this PR, but I now am pretty sure that this is merely exposing them (and they are already in use in the GitHub Actions secret anyway).
May we merge this? Sorry @craigyu for leaving your PR in limbo for 3 months...
I would suggest to move away from having shell scripts embedded in yaml files. Having an actual file ( |
Description
This PR has 3 changes:
script_stop
function. This will stop script after the first failure, and the job will show as failed. In the previous version, if for example, thegit pull
step fails, it will still run the rest of the script and we will see a green check mark thinking the job has been completed.Jira link: LF-2758
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Checklist: