MetOffice / opsinputs

JEDI library generating VarObs and Cx files
BSD 3-Clause "New" or "Revised" License
4 stars 0 forks source link

add label checker action #157

Closed yaswant closed 1 year ago

yaswant commented 1 year ago

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

What this PR does

yaswant commented 1 year ago

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 commented 1 year ago

Could the following work: https://stackoverflow.com/questions/60916931/github-action-does-the-if-have-an-else#:~:text=GitHub%20Actions%20doesn't%20have,your%20statement%20with%20%24%7B%7B%20%7D%7D%20. Two if clauses?

yaswant commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

yaswant commented 1 year ago

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

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

mikecooke77 commented 1 year ago

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

yaswant commented 1 year ago

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

Yes.