Skip to content

Refactor GHA workflows#46

Open
dskvr wants to merge 8 commits into
masterfrom
enhance/workflow-composition
Open

Refactor GHA workflows#46
dskvr wants to merge 8 commits into
masterfrom
enhance/workflow-composition

Conversation

@dskvr

@dskvr dskvr commented Feb 22, 2022

Copy link
Copy Markdown
Contributor

Lint

  • Removed Redoc build and deploy from lint

Deploy

  • Moved all env to global scope instead of job scope (DRY)
  • Created workflow that triggers on PR merge (common case) and production branch push (edge case, in the case of a force push for example)
  • Deploys packages to GHP registry.
  • Builds and deploys docs.

Notes

  • Created separate jobs for deploy, this is so that GH Pages deploy and Packages Deploy can occur in parallel. Not worth it
  • Might be better if lint were ran without building docker since it is only code formatting. The benefit to running it this way however, would be that it also confirms lint functionality via docker and aligns with user reproducibility principle @ktamas77

Todo

  • Carry artifacts between jobs in deploy Decided against separate jobs

@dskvr dskvr requested a review from ktamas77 February 22, 2022 22:15
@dskvr

dskvr commented Feb 22, 2022

Copy link
Copy Markdown
Contributor Author

On second thought, will not do separate jobs, because too much leg work would be required to carry the artifacts between jobs since we rely heavily on docker compose (only hacks can be used with docker save and compose, not worth it)

@ktamas77

ktamas77 commented Feb 23, 2022

Copy link
Copy Markdown
Contributor

Looks great, thank you for separating the jobs!
Feel free to go ahead & bring it out of draft status so we can verify both jobs passing correctly.
As a next step, I'd recommend to extend the flow in the following way (i'm going to take care of this after this one merged)

Job tree:

  1. Pull containers from registry (cache)
    -> Change? Build change layers & upload containers to registry
  2. -> Run Parallel jobs (all pulling from container registry as base):
    - Linter
    - Style check
    - API Doc Gen
    - Unit Tests

@ktamas77

ktamas77 commented Feb 23, 2022

Copy link
Copy Markdown
Contributor

On second thought, will not do separate jobs, because too much leg work would be required to carry the artifacts between jobs since we rely heavily on docker compose (only hacks can be used with docker save and compose, not worth it)

in this case, i'll go ahead and implement the tree above which could be the foundation to effectively parallelize the jobs with minimum redundancy.

@dskvr dskvr marked this pull request as ready for review February 23, 2022 13:52
@dskvr

dskvr commented Apr 8, 2022

Copy link
Copy Markdown
Contributor Author

@ktamas77 Out of draft status. We will likely need to override the Check Requirement since the workflow filename changed or add the old check into this branch to trick the Check Requirement into passing. I believe this check is hanging because GH is presently requiring a check that does not exist in this branch.

personal_token: ${{ secrets.DACOCOBOT_PAT }}
publish_dir: ./openapi
publish_branch: docs
run: docker compose run api yarn lint-prettier No newline at end of file

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[low-prio non-blocking nit-pick]

Suggested change
run: docker compose run api yarn lint-prettier
run: docker compose run api yarn lint-prettier

@ktamas77 ktamas77 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants