Skip to content

docs: add diagrams to readme#15

Merged
AnguIar merged 7 commits intomasterfrom
diagrams-to-readme
Aug 14, 2024
Merged

docs: add diagrams to readme#15
AnguIar merged 7 commits intomasterfrom
diagrams-to-readme

Conversation

@netanelC
Copy link
Copy Markdown
Contributor

This PR adds a diagram (using mermaid) to every workflow

@netanelC netanelC marked this pull request as ready for review August 13, 2024 11:09
@netanelC netanelC requested a review from a team as a code owner August 13, 2024 11:09
@netanelC netanelC requested a review from AnguIar August 13, 2024 11:09
Comment thread README.md Outdated
classDef head fill:#5882FA
classDef workflow fill:#1E4E20
A[Build And Push Docker]:::head --> B[Checkout latest commit]
B --> C(Login to Azure)
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.

Login to remote registry

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed

Comment thread README.md Outdated
classDef workflow fill:#1E4E20
A[Build And Push Docker]:::head --> B[Checkout latest commit]
B --> C(Login to Azure)
C --> D(Get Docker Image Name)
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.

It's not "get" as much as "generate"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Changed

Comment thread README.md Outdated
classDef workflow fill:#1E4E20
A[Build And Push Helm]:::head --> B[Checkout latest commit]
B --> C[Setup Helm]
C --> D[Login to Azure]
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.

Login to remote registry

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Changed

Comment thread README.md Outdated
Comment on lines +45 to +46
D --> E(Get Chart Name)
E --> F[Get Chart Version]
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.

Retrieving chart name,
Retrieving chart version

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Changed

Comment thread README.md Outdated
C --> D(Get Docker Image Name)
D --> E( Build Image)
E --> F[Push Image]
G[workflow: Edit artifacts.json in helm-charts]:::workflow
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.

I believe there should be an arrow to the "edit artifacts.json" workflow, so the reference would be more clear

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added

Comment thread README.md Outdated
E --> F[Get Chart Version]
F --> G[Package Chart into TGZ]
G --> H[Publish Chart to ACR]
I[workflow: Edit artifacts.json in helm-charts]:::workflow
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.

I believe there should be an arrow to the "edit artifacts.json" workflow, so the reference would be more clear

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added

Comment thread README.md
Comment thread README.md Outdated
D --> E[Set up Node.js]
E --> F[Install dependencies]
F --> G[Run linters]
G -->|enableOpenApiCheck==true| H["OpenAPI Lint Checks \n (on `openApiFilePath`)"]
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.

"OpenAPI Lint Checks \n (on openApiFilePath)"
I think Lint checks on OpenAPI file should be good here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Changed, although I'm not seeing a big difference

Comment thread README.md Outdated
```mermaid
flowchart TD
classDef head fill:#5882FA
A[Release On Tag Push]:::head -->|INPUTS: \n enableOpenApiToPostman| C["Checkout code for CHANGELOG.md \n (for release notes)"]
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.

Checkout Git repository is more precise

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Changed

Copy link
Copy Markdown
Contributor

@AnguIar AnguIar left a comment

Choose a reason for hiding this comment

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

💯

Comment thread README.md Outdated
F --> G[Package Chart into TGZ]
G --> H[Publish Chart to ACR]
I[workflow: Edit artifacts.json in helm-charts]:::workflow
A --> I[workflow: Edit artifacts.json in helm-charts]:::workflow
Copy link
Copy Markdown
Contributor

@AnguIar AnguIar Aug 13, 2024

Choose a reason for hiding this comment

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

Change to Trigger update-artifacts-file workflow.
Plus, arrow should come from step H, since it would be triggerd only if first job is completed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed

Comment thread README.md Outdated
C --> D(Generate Docker Image Name)
D --> E( Build Image)
E --> F[Push Image]
A --> G[workflow: Edit artifacts.json in helm-charts]:::workflow
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.

Change to "Trigger update-artifacts-file workflow".
Plus, arrow should come from step E, since it would be triggerd only if first job is completed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed

Copy link
Copy Markdown
Contributor

@AnguIar AnguIar left a comment

Choose a reason for hiding this comment

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

Looks good my dude 🥇

@AnguIar AnguIar merged commit e899d9c into master Aug 14, 2024
@AnguIar AnguIar deleted the diagrams-to-readme branch August 14, 2024 07:37
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