Skip to content

add label checker action - #157

Merged
Yaswant Pradhan (yaswant) merged 17 commits into
developfrom
feature/check_label
Feb 24, 2023
Merged

add label checker action#157
Yaswant Pradhan (yaswant) merged 17 commits into
developfrom
feature/check_label

Conversation

@yaswant

@yaswant Yaswant Pradhan (yaswant) commented Feb 22, 2023

Copy link
Copy Markdown
Collaborator

add a label checker action to validate 'ready to merge' label

What this PR does

  • trigger a Label Check action to validate 'ready to merge' label.

@ctgh Chris Thomas (ctgh) left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good! I tested removing and re-adding the label and it seemed to work well.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Great that we can do this. Could you explain why it has to be skipped if label is present in pr? Why not pass?

@yaswant

Copy link
Copy Markdown
Collaborator Author

Great that we can do this. Could you explain why it has to be skipped if label is present in pr? Why not pass?

Currently, there GitHub action does not support an else statement to tell the task has passed. I looked at adding two different tasks, but failed to trigger the action. So this was the shortest implementation, but I can research further to come up with a better solution.

@mikecooke77

Copy link
Copy Markdown
Collaborator

@yaswant

Copy link
Copy Markdown
Collaborator Author

That's what I tried in one of the commits, but the action failed to trigger. I did not investigate in detail.. Will try again.

@mikecooke77

Copy link
Copy Markdown
Collaborator

The reason I am pushing is that to make it a JCSDA acceptable solution I think it would need to be a pass rather than skipped.

Once we have failed or passed I think they could be persuaded.

@ctgh

Copy link
Copy Markdown
Collaborator

Maybe you should take off the label in order to stop a premature merge ;)

@yaswant

Copy link
Copy Markdown
Collaborator Author

I am uncertain (cant verify in this PR) if the "Check Label" badge appears for fresh PR. According to GitHub action documentation this should.

Michael Cooke (@mikecooke77) I suggest we merge the changes now and opens a separate issue if it did not function as intended.

@mikecooke77

Copy link
Copy Markdown
Collaborator

One final question before I approve. This will happen for all PRs I think not just ones to develop is that the intention?

@yaswant

Copy link
Copy Markdown
Collaborator Author

One final question before I approve. This will happen for all PRs I think not just ones to develop is that the intention?

Yes.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please merge

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.

4 participants