appliedepi / epiRhandbook_eng

The repository for the English version of the Epidemiologist R Handbook
Other
91 stars 55 forks source link

Github Action to support tracking changes and updating the handbook #155

Open ntluong95 opened 1 month ago

ntluong95 commented 1 month ago

We need to implement Github Action to support us:

1) Tracking changes across language versions of the handbook

2) Automatically re-render the handbook

Whenever changes in languages versions are approved and merged into the main branch, the book is automatically rendered and deployed in the server.

3) Periodically render the book

Periodically render the book to check for errors if some packages going out of date

robcrystalornelas commented 1 month ago

👋 Great, thanks so much for this summary @ntluong95!

So I have some ideas and I'm already implementing locally. I could just add these to my own fork of this repo, but I'm wondering if you can grant me permissions to create a branch on this repo, and open a PR that way.

Ideas / proposed changes you'll see in my PR

A new branch created with commits in the English version -> trigger Github Action to generate Branches and Pull Requests for each language (Done)

Nice!

Checkboxes linked to commits made in English version

👍 I've got a proposal for how we do this. Should be possible.

and I'm wondering if we could make the change below to just run the action every time we see a push to a branch in the repo with the format we outline here. So more based on pushes to branch rather than PRs?

- on:
-  pull_request:
-    types: [opened, reopened, synchronize]

+ on:
+   push:
+     branches:
+       - 'handbook_v*_en'

If more commits are pushed into the English branch and other language branches are already created

I think I've got a way to address this and will propose in the PR

Email notification system

Would it be sufficient to mention the assigned person in the PR that we create through this action? They should receive a notification/email depending on how their gh notifications are setup.

ntluong95 commented 1 month ago

Thanks @robcrystalornelas. I will tag @nsbatra here so he will have more details on the next step

robcrystalornelas commented 1 month ago

Sounds good! I've gone ahead and created this PR: https://github.com/appliedepi/epiRhandbook_eng/pull/156 Adding me as someone who has write access to the repo isn't totally necessary at all, but would let me test out some other GitHub features I was thinking of trying to incorporate to help us solve this issue.

Another thing @ntluong95, I suggest creating a separate issue for item number two and three above. Since those items are distinct, they may end up requiring their own PRs, and so I generally like to have more or less one PR per issue.

robcrystalornelas commented 1 week ago

👋 Hi @ntluong95 just want to follow-up here and see if we can resolve any final issues or questions with the action 🙌

Can you explain this a little more just so I help out with testing:

what I usually do is to create a separate en branch, implement changes to trigger the Action. I made some modifications in my test repo then this Action work, but the checkbox for the commit doesn't render well. Also if more commits are pushed into en branch, the PR from language versions couldn't update

So is this the general process? You could consider adding these steps to a DEV.md or CONTRIBUTING.md file in this repo as a good step to make sure we remember how to maintain these features going forward.

  1. you created a separate branch from https://github.com/appliedepi/epiRhandbook_eng or is there another en repo that I should be creating a branch in?
  2. Make some changes to one or multiple of the text files, which files make good candidates for editing?
  3. Does running the action need to happen in a test repo, or a test branch? You said test repo above, but I want to hear more about this.
  4. Then we should be able to see if the action successfully runs or not.

Also @ntluong95 could we compile the remaining to-do items here? My preference is gathering them in this issue rather than over email.

🚀 Remaining items

ntluong95 commented 1 week ago

Hi Rob,

Thank you so much for your help. So far, I am glad to tell you that I was able to setup 2 Github Actions for these purpose and they seem working well. If you see the conceptualization of the workflow in the attached image below, there is one Action to help create and update PR, and the other is used for rendering and building the book. All steps are conducted in the deploy-preview branch before merging into the main branch New workflow

  1. you created a separate branch from https://github.com/appliedepi/epiRhandbook_eng or is there another en repo that I should be creating a branch in? Currently there should be only this epiRhandbook_eng repo
  2. Make some changes to one or multiple of the text files, which files make good candidates for editing? Sure we can do that. I have my private repo where I have already tested all scenarios. But we can also test in this repo in any chapters, since they will all come to the deploy-preview branch first
  3. Does running the action need to happen in a test repo, or a test branch? You said test repo above, but I want to hear more about this. Answered above
  4. Then we should be able to see if the action successfully runs or not. So far the actions run without any problems to me. But it is always good to have expert view on it. In addition, sometimes if I just merge 1 PR, let's say in ES branch, but all other PRs were also automatically merged and closed, as well as triggering the second Github Action. It did not happen all the times. I googled and they says probably because these PRs are very identical

For remaining items

I tag @nsbatra here for his information

robcrystalornelas commented 1 week ago

@ntluong95 Awesome! Really great progress 🚀

So now that I understand your flow for testing the action, I'll give testing the action a try myself. I had a bit of time today so opened this this PR: https://github.com/appliedepi/epiRhandbook_eng/pull/173 to see if this slight refactoring of the code and different approach to getting a list of commits rather than just finding the latest commit helps us take care of Approach 2. I may have time to test it, but you should definitely give a try at testing it if you can.

As I imagine, it should describe something in the figured above right?

Exactly, I think describing the figure above and some steps about how we test the action. A document like a DEV.md is a little more "durable" than having info an issue like this. We'll close this issue eventually, and it'll be difficult to find, but there's a lot of good context you are providing here and adding it to a doc will be super helpful!

ntluong95 commented 1 week ago

@robcrystalornelas Hi Rob, It is 10pm my time now so I may not have time to test it. But will definitely look on them over the weekend.

robcrystalornelas commented 1 week ago

Of course @ntluong95 no worries at all. Have a great evening. I'm about to head out of vacation for all next week, so just working a bit on this project before then. And we can catch up when I'm back in office.

And I'll leave the testing my PR (https://github.com/appliedepi/epiRhandbook_eng/pull/173) I don't want it to inadvertently send out too many pings to all of us if it doesn't work as I expect. 😅