Skip to content

Conversation

Czaki
Copy link
Contributor

@Czaki Czaki commented Aug 1, 2023

Description

For the current state, the core dev needs to restart the dependency check, or a dummy commit needs to be done.

As the PR dependency check is cheap, I think it is worth triggering it on every comment, so if a user write comment containing merge the check will be restarted

Type of change

  • Fixes or improves workflow, documentation build or deployment

References

https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_comment-use-issue_comment

@Czaki Czaki added this to the 0.5.0 milestone Aug 1, 2023
@github-actions github-actions bot added the task label Aug 1, 2023
Copy link
Contributor

@brisvag brisvag left a comment

Choose a reason for hiding this comment

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

Nice 👍

@brisvag
Copy link
Contributor

brisvag commented Aug 1, 2023

Though maybe every comment is still a lot, since it will spin up a VM every time? shall we have a trigger word instead?

@Czaki
Copy link
Contributor Author

Czaki commented Aug 1, 2023

We may, like in napari/napari#6101. What phase did you suggest?

@brisvag
Copy link
Contributor

brisvag commented Aug 1, 2023

Yeah better. Something like "dependency closed"?

@Czaki
Copy link
Contributor Author

Czaki commented Aug 1, 2023

Maybe just "dependency"?

@brisvag
Copy link
Contributor

brisvag commented Aug 1, 2023

Though it will fire if we talk about library dependencies xD

@Czaki
Copy link
Contributor Author

Czaki commented Aug 1, 2023

hm. Maybe? I do not know. but it is now easy to suggest proper phase by "request changes"

@psobolewskiPhD
Copy link
Member

would pr merged be too easy to accidentally trigger? would accidental trigger have too high a cost?

@brisvag
Copy link
Contributor

brisvag commented Aug 31, 2023

deps can also be issues in principle I think... I think we're being overzealous here, let's just merge and if problems arise we can solve them :P

@brisvag
Copy link
Contributor

brisvag commented Aug 31, 2023

@Czaki please update description first!

@Czaki
Copy link
Contributor Author

Czaki commented Sep 5, 2023

@brisvag updated description

@brisvag brisvag merged commit 1a1f371 into napari:main Sep 5, 2023
@Czaki Czaki deleted the pr_depenedcy_on_comment branch September 5, 2023 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants