ci: notify non generated dist on PR

[zekth]

Add verification step for: #62

We can setup an automatic generation on pushing to the default branch, but for this we need a Personnal Access Token to be created to allow to push on the repo directly from CI. To be honest i prefer adding a verification step to ensure collaborators to run npm run build even if there is a husky hook

Checklist

• run npm run test and npm run benchmark
• tests and/or benchmarks are included
• documentation is changed or added
• commit message and code follows the Developer's Certification of Origin
  and the Code of conduct

[JamesKyburz]

If we do it on push to main we can use the default GITHUB_TOKEN.

[zekth]

If we do it on push to main we can use the default GITHUB_TOKEN.

Doesn't this require you to have permission on the default branch + no push restriction on the branch? I haven't set this up on public repos.

[JamesKyburz]

If we do it on push to main we can use the default GITHUB_TOKEN.

Doesn't this require you to have permission on the default branch + no push restriction on the branch? I haven't set this up on public repos.

Yes you're right when the branch is protected.
I also think the verification step is simpler, however it doesn't take dependabot merges into account.

[mcollina]

lgtm

[simoneb]

are you sure this is the name the actor gets when it runs as dependabot? I can think of more exceptions though. not all files end up generating a different dist folder contents. say changes to readme or anything else that is not source code really. are we sure it's safe to break the build just for the sake of somebody not running husky hooks properly? I'm not convinced honestly.

[climba03003]

I am +1 for simoneb stance. In addition, as a github action release need manually change the package.json version. It is not a good idea to force updating dist file without the change of package version.

[zekth]

for example hooks do not run on my env (some projects running in WSL makes it crash), it's more convinient to ensure this in the CI. See: #61 (review)

Regarding dependabot AFAIK this is its account name.

Changes not in source code won't change the build artifact. On the other hand, dependabot will not run the build command and in the case of a dependency change we'll have to run it on our own.

The easiest thing is to run this on commit to default but like explain it can be a pain. Otherwise we can ship a Dockerfile which would be consumed with the build inside of it, but for our case it's kind of overkill.

[zekth]

I am +1 for simoneb stance. In addition, as a github action release need manually change the package.json version. It is not a good idea to force updating dist file without the change of package version.

A lot of people rely a non tagged actions, which means checking out the default branch. So in this case it means you're adding commits to the history without updating the code that the action will run. In the end the artifact of the default branch will not be in sync with the code.

[simoneb]

I'm sorry but I'm not in favor of this change. I'm all for automation when it removes work, but I'm not when it increases the amount of work you have to do.

• Without this automation we need to make sure that the dist folder is generated, which clearly can be a source of errors but it won't for most people because
  • husky is a fairly standard tool and most setups will run it correctly
  • missing dist/ changes will hopefully be caught by PR reviews
• With this automation some commits will start failing for no good reasons, and this to me is more maintenance burden than the first point

[simoneb]

Note that part of the issue might be that the husky script was not executable #61 (comment)