search

lsms-worldbank / adodown

Tools for building Stata package documentation websites
https://lsms-worldbank.github.io/adodown/
5 stars 0 forks source link

Remove PR trigger from template GH action workflow #4

Open kbjarkefur opened 1 year ago

kbjarkefur commented 1 year ago

https://github.com/lsms-worldbank/adodown/blob/4e9ab1204aee5a907a67c93da7613f7e5537dfc1/src/ado/templates/ad-build_adodown_site.yaml#L1-L5

I suggest changing this to:

 on: 
   push: 
     branches: [main, master]

A merge of a PR is considered a push. So the action will be triggered when a PR is merged to main/master.

Otherwise the action will be triggered when a PR towards main/master is opened, and when a branch with an open PR towards main/master is updated. While technically not an issue, it is redundant, as in those cases nothing has changed on main/master where the action reads to, so it will never lead to any changes on the gh-pages branch. Since GH actions are free on public repos, this is not an issue in most cases, but if, for example, the build fails, you will get an error notification each time you update an open PR.

So all-in-all, I would say it is cleaner to not have a trigger for PRs. What do you think @arthur-shaw ?

kbjarkefur commented 1 year ago

If we agree to remove this, then I guess we should also remove this line:

https://github.com/lsms-worldbank/adodown/blob/4e9ab1204aee5a907a67c93da7613f7e5537dfc1/src/ado/templates/ad-build_adodown_site.yaml#L67

kbjarkefur commented 1 year ago

Done in f77d89894c4e4aa53e538c884b7fbacd8ea1d604

Note the file was also renamed in that commit.

Leaving this issue open until our team has decided that this is what we will do.