Skip to content
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.

Sign up for GitHub

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

Jump to bottom

GitHub actions workflow template pipeline #246

Merged
merged 13 commits into from
Jun 14, 2024
Merged

GitHub actions workflow template pipeline #246

merged 13 commits into from
Jun 14, 2024

Conversation

johnmnemec
Copy link
Contributor

This pull request is for adding the GitHub Actions pipeline template.

@johnmnemec
Copying pipeline over
3c75de3 
Copying over the GitHub Actions pipeline to the DBB repo

Signed-off-by: johnmnemec <86624614+johnmnemec@users.noreply.github.com>
@johnmnemec
Polishing and refactoring
bb87361 
Various tweaks and changes

Signed-off-by: johnmnemec <86624614+johnmnemec@users.noreply.github.com>
@johnmnemec
Updated readme diagrams
aa8530b 
Signed-off-by: johnmnemec <86624614+johnmnemec@users.noreply.github.com>
@johnmnemec
Final code tweaks
ecf45b1 
Signed-off-by: johnmnemec <86624614+johnmnemec@users.noreply.github.com>
@johnmnemec
Cleanup
ce99170 
Minor tweaks before review

Signed-off-by: johnmnemec <86624614+johnmnemec@users.noreply.github.com>
@dennis-behm dennis-behm marked this pull request as ready for review May 24, 2024 14:28
@johnmnemec
Updating placeholder values
4839ac4 
Signed-off-by: johnmnemec <86624614+johnmnemec@users.noreply.github.com>
@johnmnemec
Tweaking
c0b7d6b 
Moved workflow files to a pipeline folder to help with organization

Updated the readme to include a link to documentation for manually pausing and resuming a workflow.

Signed-off-by: johnmnemec <86624614+johnmnemec@users.noreply.github.com>
@johnmnemec
Minor Tweaks
0becbcc 
Fixed an incorrect pipelineType, and removed unnecessary whitespace

Signed-off-by: johnmnemec <86624614+johnmnemec@users.noreply.github.com>
dennis-behm
dennis-behm reviewed May 28, 2024
View reviewed changes
Copy link
Member

@dennis-behm dennis-behm left a comment

Choose a reason for hiding this comment

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

@johnmnemec I have put in a few comments, but I am not yet done with the review. Would you mind to take care of them, please?

Templates/GitHubActionsPipeline/.github/actions/tagging_createreleasecandidate/action.yml Outdated Show resolved Hide resolved
Templates/GitHubActionsPipeline/.github/workflows/pipelines/Build.yml Outdated Show resolved Hide resolved
Templates/GitHubActionsPipeline/.github/workflows/pipelineController.yml Show resolved Hide resolved
Templates/GitHubActionsPipeline/.github/workflows/pipelines/Preview.yml Outdated Show resolved Hide resolved
Templates/GitHubActionsPipeline/.github/workflows/pipelines/Feature.yml Outdated Show resolved Hide resolved
@johnmnemec
Various fixes for PR
ce2e833 
- Removed the extra workflows/pipeline/ folder, as workflow files must be in the root of the workflow folder
- removed unnecessary prints and comments
- removed "-t '--fullBuild'" from the build step in Build.yml
- Changed the pipelineType input type from string to choice
- Added a cleanup stage to Feature.yml and Preview.yml

Signed-off-by: johnmnemec <86624614+johnmnemec@users.noreply.github.com>
@johnmnemec
Copy link
Contributor Author

@johnmnemec I have put in a few comments, but I am not yet done with the review. Would you mind to take care of them, please?

I've addressed all of those, thanks!

@dennis-behm
Copy link
Member

@johnmnemec I am testing this on our internal test system. I noticed the following, which we could address. It's about the presentation of the pipeline.

The build has the following steps:
image

The deployment includes the prepare log step.
image

Instead of having a dedicated stage build / PrepareBuildLogs, can we include that into the Build phase?

dennis-behm
dennis-behm reviewed Jun 4, 2024
View reviewed changes
Copy link
Member

@dennis-behm dennis-behm left a comment

Choose a reason for hiding this comment

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

@johnmnemec Preparing and publishing the build logs should always occur.

Templates/GitHubActionsPipeline/.github/workflows/Build.yml Outdated Show resolved Hide resolved
Templates/GitHubActionsPipeline/.github/workflows/Release.yml Outdated Show resolved Hide resolved
dennis-behm
dennis-behm reviewed Jun 4, 2024
View reviewed changes
Copy link
Member

@dennis-behm dennis-behm left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@johnmnemec

  • Creation of the release candidate tag is a bit too early in the process (imho) and would better fit into the Packaging stage.

@dennis-behm
Copy link
Member

@johnmnemec

  • Creation of the release candidate tag is a bit too early in the process (imho) and would better fit into the Packaging stage.

@johnmnemec Sorry. Forget about it. I got confused. It is in the right order!

Let's focus on this comment first: #246 (comment)

@monvora
Copy link
Member

monvora commented Jun 5, 2024

@johnmnemec Hello John, we need to update the way we retrieving the latest release candidate tag (on line # 91) as below.

releaseCandidate=$(git tag --list "${newVersionTag}_*rc*" --sort=version:refname | tail -n 1)

We need to sort by version:refname instead of committerdate since we cannot relay on committerdate. I found inconsistency behavior when trying to retrieve the latest RC tag from committerdate.

@johnmnemec
Copy link
Contributor Author

@johnmnemec I am testing this on our internal test system. I noticed the following, which we could address. It's about the presentation of the pipeline.

The build has the following steps: image

The deployment includes the prepare log step. image

Instead of having a dedicated stage build / PrepareBuildLogs, can we include that into the Build phase?

My only concern with this is that merging the Build and PrepareBuildLogs stages in the Build pipeline would then cause the formatting of the build pipeline to be inconsistent with the rest of the pipelines, and I'd prefer to avoid that.

@johnmnemec
Fixes and tweaks for PR
6aebcd5 
Addressed various changes as requested by Dennis and Molly.

Signed-off-by: johnmnemec <86624614+johnmnemec@users.noreply.github.com>
@johnmnemec
Consolidating pipeline stages
8667605 
Merging the prepareBuildLogs stage into the Build stage on the Build, Feature, and Release pipeline

Signed-off-by: johnmnemec <86624614+johnmnemec@users.noreply.github.com>
@johnmnemec
Copy link
Contributor Author

@johnmnemec I am testing this on our internal test system. I noticed the following, which we could address. It's about the presentation of the pipeline.
The build has the following steps: image
The deployment includes the prepare log step. image
Instead of having a dedicated stage build / PrepareBuildLogs, can we include that into the Build phase?

My only concern with this is that merging the Build and PrepareBuildLogs stages in the Build pipeline would then cause the formatting of the build pipeline to be inconsistent with the rest of the pipelines, and I'd prefer to avoid that.

I've taken another look at this, and cleaned it up properly

@dennis-behm FYI

@johnmnemec
Merge branch 'main' into feature/GithubActionsTemplate
64286ac
@johnmnemec
Fixed conditionals and images
51265a3 
Adjusted the conditional for the build pipeline in the pipelineController

Updated the pipeline preview images

Signed-off-by: johnmnemec <86624614+johnmnemec@users.noreply.github.com>
dennis-behm
dennis-behm approved these changes Jun 14, 2024
View reviewed changes
Copy link
Member

@dennis-behm dennis-behm left a comment

Choose a reason for hiding this comment

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

Thank you @johnmnemec great addition!

@dennis-behm dennis-behm merged commit 3cb7d85 into IBM:main Jun 14, 2024
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dennis-behm dennis-behm dennis-behm approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@johnmnemec @dennis-behm @monvora