Skip to content
This repository has been archived by the owner on Nov 20, 2024. It is now read-only.

Removed deployment job, refactored tests and updated CI image. #23

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

AlexSkrypnyk
Copy link

@AlexSkrypnyk AlexSkrypnyk commented Jun 11, 2020

  • Updated Docker image to use CI image hosted on GitHub.
  • Removed deploy job as deployment is manual. The current configuration was confusing. The deployment should only be manual to respect git-flow.
  • Refactored tests into a separate file to allow running locally.
  • Split CI into 3 jobs to speed-up tests (~10 mins to ~2mins)
  • Updated documentation formatting:
    • Added more inline code blocks
    • Fixed line length to fit into 80 characters

@AlexSkrypnyk AlexSkrypnyk self-assigned this Jun 11, 2020
@AlexSkrypnyk AlexSkrypnyk requested a review from steveworley June 11, 2020 11:23
@AlexSkrypnyk AlexSkrypnyk force-pushed the feature/remove-deploy branch from 878ef0f to 9aad536 Compare June 11, 2020 12:18
@AlexSkrypnyk AlexSkrypnyk changed the title Removed deploy job as deployment is manual + updated Docker image. Removed deployment job, refactored tests and updated CI image. Jun 11, 2020
@AlexSkrypnyk AlexSkrypnyk requested a review from stooit June 11, 2020 12:23

# shellcheck disable=SC2002,SC2015

# Satis application directory.
Copy link
Author

Choose a reason for hiding this comment

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

as usual, the first section is expected variables


#-------------------------------------------------------------------------------

echo "--> Starting Satis web server on http://localhost:4141."
Copy link
Author

Choose a reason for hiding this comment

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

this part allows to kill previously started inbuilt php server from the previous run, start the server, wait for app to be ready and assess that the server was successfully started

echo "--> Add http://localhost:4141/${SATIS_BRANCH} as a repository."
composer --working-dir="${GOVCMS_SCAFFOLD_DIR}" config repositories.govcms composer http://localhost:4141/"${SATIS_BRANCH}"

if [ "${SATIS_BRANCH}" = "master" ] || [ "${SATIS_BRANCH}" = "develop" ] ; then
Copy link
Author

Choose a reason for hiding this comment

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

the logic below has not been changed. only verbose output added

@@ -1,140 +1,28 @@
version: 2.1
parameters:
Copy link
Author

Choose a reason for hiding this comment

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

cleaned up CI as we are only generating content manually. once there is a better automated process - the config can be revert. otherwise - this is a dead code.

- develop
requires:
- test
- test
Copy link
Author

Choose a reason for hiding this comment

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

3 parallel jobs allows to reduce build time to ~2 mins and make sure that there are no artifacts from the previous build (clean run)


Currently, updating is manual. The job to automate it was completed but it got stuck waiting
for deployment tokens. In the meantime it has been found that manual intervention is usually needed.
Updating of the Satis repository content is a manual process that includes
Copy link
Author

Choose a reason for hiding this comment

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

this is the only paragraph that was changed to explain what the process is.

The rest of the changes are formatting only.

@simesy
Copy link
Contributor

simesy commented Jun 12, 2020

Sweet, I was hesitant to remove all the automation without knowing the roadmap. This will be a clean change and it can serve as a reference if automation is ever re-added.

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.

2 participants