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

Label to title #125

Merged
merged 16 commits into from
Aug 14, 2024
Merged

Label to title #125

merged 16 commits into from
Aug 14, 2024

Conversation

samwaseda
Copy link
Member

I did more or less what @liamhuber said here

@samwaseda
Copy link
Member Author

I'm testing it here but so far it's not working.

@liamhuber
Copy link
Member

I'm testing it here but so far it's not working.

I'm on mobile, but it looks like the test location needs the "uses" clause nested inside the regular "steps" field; take a look at pyiron workflow or some of the actions reusable workflows for something to mimic

@samwaseda
Copy link
Member Author

The other ones use yml files from .github/workflow/xyz.yml. Should I make a file there?

@liamhuber
Copy link
Member

No, just invoke it like a usual action invocation:

- uses: actions/checkout@v4

@liamhuber
Copy link
Member

Showing my work: I grabbed the github.token recommendation from ChatGPT, but it looks super familiar and I was hunting for that answer, so I think I read it in the docs. The shell lines are just copy-pasted from the other places we use the shell.

samwaseda and others added 5 commits August 14, 2024 08:03
Co-authored-by: Liam Huber <liamhuber@greyhavensolutions.com>
Co-authored-by: Liam Huber <liamhuber@greyhavensolutions.com>
Co-authored-by: Liam Huber <liamhuber@greyhavensolutions.com>
Co-authored-by: Liam Huber <liamhuber@greyhavensolutions.com>
Co-authored-by: Liam Huber <liamhuber@greyhavensolutions.com>
@liamhuber
Copy link
Member

Alternatively, we could take the token as explicit input like the checkout action does:

- uses: actions/checkout@v4
with:
token: ${{ secrets.DEPENDABOT_WORKFLOW_TOKEN }}

@liamhuber
Copy link
Member

Actually, that's probably wiser -- we might get away with secrets being automatically inherited when we use the action inside the pyiron org, but it is just generally a great action and we want it available globally.

@liamhuber
Copy link
Member

pyiron/elaston#18 (comment)

But don't merge yet, let's try from a repo outside the org to make sure the secret still passes

@liamhuber
Copy link
Member

liamhuber commented Aug 14, 2024

Hmm, runs but doesn't fail hard when I try it in my own namespace: https://github.com/liamhuber/action-test/actions/runs/10382454910/job/28745564689?pr=2

EDIT: runs but doesn't work, critically

@liamhuber
Copy link
Member

Even with the token getting passed in, I'm still getting the same access error from outside the org:

 {
  "message": "Resource not accessible by integration",
  "documentation_url": "https://docs.github.com/rest/issues/issues#update-an-issue",
  "status": "403"
}

I've got to go to bed, but I'll be available tomorrow at some point in the AM to take another look

@liamhuber
Copy link
Member

liamhuber commented Aug 14, 2024

Ahh, ok, the problem was on the side of the repo, not the side of the action! I hadn't configured the repo to allow actions to modify stuff:

Screen Shot 2024-08-14 at 09 15 55

With that done it's working fine!

I want to try:

  • Reverting the token-input changes to see if it works
    • If it then needs to inherit secrets, using the token to pass only the secret it needs seems preferable (and in line with actions/checkout)
  • Use step outputs instead of the deprecated set-output

Based on something that hung me up
Copy link
Member

@liamhuber liamhuber left a comment

Choose a reason for hiding this comment

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

@samwaseda, nice! I'm happy with it. The problems I was running into last night were entirely on my end -- repo settings were off. I fixed the deprecation warning and added some comments to help users. Take a look at my changes, but from my end this is good, working as intended, and useful.

Extensions that make sense to me:

  • Proof-check that the label doesn't already start with "[TAG]" -- right now, if you add the "patch" label to a title already starting with "[patch]" you get "[patch] [patch] ...". IMO, ideal behaviour would be for it to not act if the title already starts with the new prefix, and to fail if it starts with a different prefix
  • Run the action in reverse, i.e. if the label is updated and it starts with a "[TAG]", add the corresponding label
  • Make it flexible, so that tags are space-separated input with defaults of "patch major minor" so it can add arbitrary title prefixes

But with the core functionality working well, IMO those can be left out of this PR

@samwaseda
Copy link
Member Author

Yeah I thought about the first point. For the second point I got the feeling that it’s more important to have the tag in the title than as a label so I guess we can leave it for now.

@samwaseda samwaseda merged commit ec00101 into main Aug 14, 2024
@samwaseda samwaseda deleted the label-to-title branch August 14, 2024 16:54
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.

2 participants