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.

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

feat(controller): Create GitHub check for taskRuns #146

Merged
merged 2 commits into from
Jun 30, 2022

Conversation

SamoKopecky
Copy link
Member

@SamoKopecky SamoKopecky commented Jun 17, 2022

Resolves #63
Resolves #51

A GitHub check is created whenever a taskRun is created. The user can then re-run the task via the check re-run button. The checks are created only if the push contains a change in peribolos.yaml config, if more commits are pushed at the same time the check runs only on the most recent commit.

@sesheta sesheta added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 17, 2022
@sesheta
Copy link
Member

sesheta commented Jun 17, 2022

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@sesheta sesheta added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jun 17, 2022
@SamoKopecky SamoKopecky marked this pull request as ready for review June 20, 2022 10:51
@sesheta sesheta removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 20, 2022
@SamoKopecky SamoKopecky changed the title feat(controller): Add issue creation on taskRun fail feat(controller): Create issue on taskRun fail Jun 20, 2022
@SamoKopecky SamoKopecky changed the title feat(controller): Create issue on taskRun fail feat(controller): Create issue on taskRun fail and re-run capability Jun 20, 2022
@SamoKopecky SamoKopecky changed the title feat(controller): Create issue on taskRun fail and re-run capability feat(controller): Create issue on taskRun fail and add re-run capability Jun 20, 2022
@SamoKopecky SamoKopecky force-pushed the create-issue-on-task-fail branch 2 times, most recently from 1e115c1 to b02a0b0 Compare June 20, 2022 15:08
Copy link
Member

@tumido tumido left a comment

Choose a reason for hiding this comment

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

Quick review, I think we're not heading in the right direction with this. We should not be blocking the controller and keeping it occupied while Tekton is processing a task.

src/app.ts Outdated Show resolved Hide resolved
src/app.ts Outdated Show resolved Hide resolved
src/app.ts Outdated Show resolved Hide resolved
@tumido
Copy link
Member

tumido commented Jun 21, 2022

Btw, those issues created by a failed workflow, I love it! Those log snippets are great! 👍 💯

@SamoKopecky SamoKopecky changed the title feat(controller): Create issue on taskRun fail and add re-run capability feat(controller): Create GitHub check on taskRuns Jun 23, 2022
@SamoKopecky SamoKopecky changed the title feat(controller): Create GitHub check on taskRuns feat(controller): Create GitHub check for taskRuns Jun 23, 2022
@SamoKopecky
Copy link
Member Author

/cc @tumido

Copy link
Member

@tumido tumido left a comment

Choose a reason for hiding this comment

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

Great progress, I like it, there are some little things to fix but overall I like this.

src/app.ts Outdated Show resolved Hide resolved
src/app.ts Outdated Show resolved Hide resolved
src/app.ts Outdated Show resolved Hide resolved
src/app.ts Outdated Show resolved Hide resolved
src/app.ts Show resolved Hide resolved
@SamoKopecky
Copy link
Member Author

SamoKopecky commented Jun 27, 2022

Updated with a pushed commit.
/cc @tumido

Example of as skipped check_run:

src/app.ts Outdated Show resolved Hide resolved
src/app.ts Outdated Show resolved Hide resolved
@SamoKopecky
Copy link
Member Author

New changes were pushed. Ready for review again!

Copy link
Member

@tumido tumido left a comment

Choose a reason for hiding this comment

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

Great! We're getting there! 🙂 I have one small nit for you, otherwise lgtm 👍

src/app.ts Outdated Show resolved Hide resolved
@SamoKopecky SamoKopecky force-pushed the create-issue-on-task-fail branch 2 times, most recently from cd26b9e to 2dc3e04 Compare June 30, 2022 09:19
@SamoKopecky
Copy link
Member Author

SamoKopecky commented Jun 30, 2022

Thanks for the great feedback Tom, ready for review.

I think we should wait for #156 to merge since we are migrating the tasks to the new secret name. I can then rebase my branch to the upstream and rename this secret reference too.

@SamoKopecky SamoKopecky requested a review from tumido June 30, 2022 09:21
Copy link
Member

@tumido tumido left a comment

Choose a reason for hiding this comment

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

Last issue, I promise 😄 🤞

src/app.ts Outdated Show resolved Hide resolved
@SamoKopecky
Copy link
Member Author

/hold
Waiting for #156 to merge.

@sesheta sesheta added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 30, 2022
@SamoKopecky
Copy link
Member Author

/unhold
Rebased to upstream and referencing the correct secret in run task.

@sesheta sesheta removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 30, 2022
Copy link
Member

@tumido tumido left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@sesheta sesheta added the lgtm Indicates that a PR is ready to be merged. label Jun 30, 2022
@sesheta
Copy link
Member

sesheta commented Jun 30, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: tumido

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@sesheta sesheta added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 30, 2022
@sesheta sesheta merged commit 24b25e9 into operate-first:main Jun 30, 2022
github-actions bot pushed a commit that referenced this pull request Jul 7, 2022
# [1.2.0](v1.1.0...v1.2.0) (2022-07-07)

### Bug Fixes

* **tasks:** Reference the correct secret ([abb2957](abb2957))

### Features

* **controller:** Create GitHub check for taskRuns ([#146](#146)) ([24b25e9](24b25e9))

### Reverts

* Reset dev overlay to base ([3b503ec](3b503ec))
@github-actions
Copy link
Contributor

github-actions bot commented Jul 7, 2022

🎉 This PR is included in version 1.2.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@github-actions github-actions bot added the released Marks issues as released label Jul 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. released Marks issues as released size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create issue to user if peribolos is unable to apply config How to re-trigger a failed peribolos workflow
3 participants