americanhandelsociety / americanhandelsociety-members

0 stars 0 forks source link

Add github workflow #165

Closed reginafcompton closed 1 year ago

reginafcompton commented 1 year ago

This PR adds a Github workflow that does the following:

The workflow runs when pushing a commit to a branch against main.

Questions:

reginafcompton commented 1 year ago

@ecedmondson - there's not to review here. Can you: (1) consider my questions, and (2) identify anything critical I might be overlooking in our CI pipeline.

ecedmondson commented 1 year ago

I know this has been lingering...I have been very busy and have not yet had a chance to look at it. I am planning to try and carve out some time Wednesday evening. I hope that's alright! Thank you for the patience.

ecedmondson commented 1 year ago

should we run this when pushing to main? I suppose so.

I thought that was the point of this, so, I think so.

is there a way to block merging until this passes? or is that a paid feature? Looks like that is a paid feature: https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/managing-protected-branches/managing-a-branch-protection-rule

I found these: https://docs.github.com/en/actions/learn-github-actions/expressions

I wondered if we could come up with something hacky using always() or success() to prevent merges...but I assume if branch protection rules are a paid feature that the folks who implemented the paid feature wouldn't permit a hack around...

Some other thoughts I had, extremely non-blocking, is to implement a job that would conditionally rerun if certain tests (flaky, not that we have any, I don't think) fail with a keyword. We did this at RS, so, for example, if a job flaked we would comment "rerun please" and the comment event would trigger a rerun. We might be able to do something like that with this: https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#issue_comment

reginafcompton commented 1 year ago

a job that would conditionally rerun if certain tests (flaky, not that we have any, I don't think) fail with a keyword.

ooooooooooooooohh. I like this.

we should add a flaky test so we can do this.

ecedmondson commented 1 year ago

we should add a flaky test so we can do this.

A nonsense flaky test...right?

reginafcompton commented 1 year ago

I opened a ticket for the stick-it-to-the-man-because-we-are-a-non-profit hack: https://github.com/americanhandelsociety/americanhandelsociety-members/issues/166

ecedmondson commented 1 year ago

A trivial change to test CI once more

That documentation seems useful, but in the future if you don't have anything useful to add, you can do empty commits (I am sorry if I am telling you something you already know)

git commit --allow-empty -m "I love empty commits!"

reginafcompton commented 1 year ago

Closes #54

reginafcompton commented 1 year ago

you can do empty commits

oh I know! I actually had this change stashed because I noticed that weird indentation issue a little while ago.

ecedmondson commented 1 year ago

oh I know! I actually had this change stashed because I noticed that weird indentation issue a little while ago.

I apologize for overexplaining!