openstreetmap / id-tagging-schema

🆔🏷 The presets and other tagging data used by the iD editor
ISC License
156 stars 157 forks source link

fix broken CI script #1254

Closed k-yle closed 4 months ago

k-yle commented 4 months ago

See https://github.com/JamesIves/github-pages-deploy-action/discussions/1589 for more info. The 'deploy' pipeline has been broken for the past 2 weeks

github-actions[bot] commented 4 months ago

:bento: You can preview the tagging presets of this pull request here.

tordans commented 4 months ago

Only way to test it is to merge, right?

Well, not really :-D

This job was skipped

Maybe something checks if the right files where changed and only then allows the Action to run. But re-running the last action https://github.com/openstreetmap/id-tagging-schema/actions/workflows/deploy.yml does not work either, it shows the same error (I thought it would use the current code to run but maybe it uses the specific commit…).

=> We will just have to wait until we merge something else, I guess.

tordans commented 4 months ago

@k-yle the prettier merge just now ran the script in https://github.com/openstreetmap/id-tagging-schema/actions/runs/9411968617 and it still fails. Did not look into it more.

Btw, there is another info about outdated node being used by the action which we fixed with https://github.com/FixMyBerlin/atlas-app/commit/e9e260fbdc0dfa29039f5e5324029f32db3756e7 in a different project. We might want to lookout for that as well.

k-yle commented 4 months ago

@tordans it's a different error now, because pushing directly to the main branch is no longer allowed:

image

One solution is to make the bot automatically opens a PR instead of committing to main, but @tyrasd should probably make a decision about what to do.

For completeness: another solution would be to store the dist files in a dedicated branch, instead of a folder on the main branch. This is the more standard, but it would be a major breaking change

tordans commented 4 months ago

pushing directly to the main branch is no longer allowed

Martin can probably add the Action to an allow list that allows it to push to main and bypass the branch protection.

k-yle commented 3 months ago

Martin can probably add the Action to an allow list that allows it to push to main and bypass the branch protection.

I'm interested to know if this is possible, because last time I checked, it was only possible to make exceptions for Users/Bots/GitHub Apps, but not GitHub Actions^1, since actions use the repository's GITHUB_TOKEN.

tyrasd commented 1 month ago

Martin can probably add the Action to an allow list that allows it to push to main and bypass the branch protection.

I did a bit of research and as far as I see, there is no good way to make this work (see also this short discussion on the repo of the github action we use in the workflow), at least not easily. :shrug:

We could either disable the branch protection rules again (and manually check that our "branch/PR rules" are followed), or otherwise make sure that the necessary "deploy" routine is performed when each PR is merged.

My quick opinion would be that the second option might be more error-prone in the long term than the first. What do you think?

tyrasd commented 1 month ago

A third option would be to do the "deploy" of the interim folder into a separate branch, which would not have to be protected. Then we only need to adjust the respective scripts in iD and transifex to point to the new branch. That's probably the better solution here.

//edit: iD and transifex now point to the new branch for this. next step would be to drop the interim directory in the main branch: let's do this after the upcoming release