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

CI: Add labels to issue and PR and improve output from submission wf #481

Merged
merged 50 commits into from
Aug 17, 2023

Conversation

mickahell
Copy link
Collaborator

@mickahell mickahell commented Aug 6, 2023

Summary

Add labels to issue and PR from submission wf.
Improve the wf output.

Details and comments

  • submission label to every submission issue and PR
  • on hold label to every submission issue and PR who doesn't pass the standard check
  • remove on hold label if the submission passed
  • ready label to every submission issue and PR who passed the standard check
  • remove ready label if the submission doesn't passed anymore
  • Add a wf summary
  • Make the comment in the issue more clean
  • Make the comment in the PR more clean
  • Tests run
  • Clean

Closes #480

@CLAassistant
Copy link

CLAassistant commented Aug 6, 2023

CLA assistant check
All committers have signed the CLA.

@mickahell mickahell marked this pull request as draft August 6, 2023 10:29
@mickahell mickahell self-assigned this Aug 6, 2023
@mickahell mickahell added the enhancement New feature or request label Aug 6, 2023
@mickahell mickahell changed the title CI: Add labels to issue and PR from submission wf CI: Add labels to issue and PR and improve output from submission wf Aug 6, 2023
@mickahell
Copy link
Collaborator Author

@frankharkins ready for your review :)

I also implemented a new comment structured using the GitHub step summary. That's allow to simplify the code and have more clear output :)
Capture d’écran 2023-08-06 à 17 17 20

All the examples :

(the 2 errors in the annotations are normal it's because I'm asking to delete labels who aren't on the issue, I added a continue-on-error: true in order to ignore them)

@mickahell mickahell marked this pull request as ready for review August 6, 2023 15:23
mickahell and others added 15 commits August 6, 2023 17:40
* force string for output

* Revert "force string for output"

This reverts commit cabecb9.

* make stable and dev check informational + badge good when only standard

* set messages

* update unit tests

* lint

* lint

* fork doesn't sync

* fork sync

* Update .github/workflows/ecosystem-submission.yml

Co-authored-by: Frank Harkins <[email protected]>

Co-authored-by: Frank Harkins <[email protected]>

* add assert for set_actions_output
Co-authored-by: Frank Harkins <[email protected]>

* Revert "add assert for set_actions_output"

This reverts commit 51c0f56.

* Update .github/workflows/ecosystem-submission.yml

Co-authored-by: Frank Harkins <[email protected]>

* Update .github/workflows/ecosystem-submission.yml

Co-authored-by: Frank Harkins <[email protected]>

* no regex in actions if

* clear condition

---------

Co-authored-by: Frank Harkins <[email protected]>
Badges and stars update
Time: 2023_08_02_13_46

Co-authored-by: frankharkins <[email protected]>
@mickahell mickahell force-pushed the feature/pr-labels-submission branch from 71334fc to e20f586 Compare August 6, 2023 15:57
@mickahell
Copy link
Collaborator Author

mickahell commented Aug 6, 2023

The long line of commits was because I had to amended a commit from GitHub action ^^'

Copy link
Member

@frankharkins frankharkins left a comment

Choose a reason for hiding this comment

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

Look great, thanks!

Only problem is I think a couple of steps are duplicated. I've also suggested some name / comment changes to make things clearer at a glance.

.github/workflows/ecosystem-submission.yml Outdated Show resolved Hide resolved
.github/workflows/ecosystem-submission.yml Outdated Show resolved Hide resolved
.github/workflows/ecosystem-submission.yml Outdated Show resolved Hide resolved
.github/workflows/ecosystem-submission.yml Outdated Show resolved Hide resolved
.github/workflows/ecosystem-submission.yml Outdated Show resolved Hide resolved
.github/workflows/ecosystem-submission.yml Outdated Show resolved Hide resolved
.github/workflows/ecosystem-submission.yml Outdated Show resolved Hide resolved
.github/workflows/ecosystem-submission.yml Outdated Show resolved Hide resolved
.github/workflows/ecosystem-submission.yml Outdated Show resolved Hide resolved
.github/workflows/ecosystem-submission.yml Outdated Show resolved Hide resolved
@mickahell
Copy link
Collaborator Author

Look great, thanks!

Only problem is I think a couple of steps are duplicated. I've also suggested some name / comment changes to make things clearer at a glance.

Yep @frankharkins I took your feedbacks :)
The duplicated aren't, they are about PR or issue the blocks are almost the same except the variable for the issue id

Copy link
Member

@frankharkins frankharkins left a comment

Choose a reason for hiding this comment

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

Sorry to request more changes. I now understand there's a separate step for each label addition/removal; it might be clearer to combine these eight steps into two steps.

That is, this pattern

if condition:
    do x
    do y
    do z

rather than this one

if condition
    do x
if condition
    do y
if condition
    do z

Then there should be less code and it should be harder to miss something when modifying the action.

.github/workflows/ecosystem-submission.yml Outdated Show resolved Hide resolved
.github/workflows/ecosystem-submission.yml Outdated Show resolved Hide resolved
@mickahell
Copy link
Collaborator Author

Hmm ok I see what you mean, I'm not sure I can merge stps together. I'l check that

@frankharkins
Copy link
Member

Can you just put all the code in the same script? E.g.

with:
  script: |
    github.rest.issues.addLabel({
      ...
    })
    github.rest.issues.removeLabel({
      ...
    })

@mickahell
Copy link
Collaborator Author

Can you just put all the code in the same script? E.g.

Totally ! I was somewhere else and totally forget I can merge script together ^^'

Sorry to request more changes. I now understand there's a separate step for each label addition/removal; it might be clearer to combine these eight steps into two steps.

Don't worry that's totally ok :)

Done, I mixed them. You can review it

Copy link
Member

@frankharkins frankharkins 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, thank you!

@frankharkins frankharkins merged commit fb86c84 into Qiskit:main Aug 17, 2023
4 checks passed
@mickahell mickahell deleted the feature/pr-labels-submission branch August 18, 2023 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add label to issue and PR during submission wf and improve output
3 participants