ci(workflow): post build result as comment

[SethFalco]

What does this PR do?

Improve repo

Description

• ~Removed .travis.yml as we don't need Travis CI anymore.~ Adressed in: #5591
• ~Appends .editorconfig with 2 space indentations for *.yml files.~ Adressed in: #6696
• Updates the workflow to post a review and comment on the result of the build.
  • On push, if the build fails it will post the output of fpb-lint and request changes.
  • On push, if the build succeeds it will just approve the changes.

If possible, it would be better if this was not merged as is, but we waited my PR to do fpb-lint to merge first:

• https://github.com/vhf/free-programming-books-lint/pull/29

Then I can make a change to this PR like:

- - run: |
-   git clone https://github.com/SethFalco/free-programming-books-lint.git
-   cd free-programming-books-lint
-   git checkout origin/multiple-dirs
-   npm i
-   npm i -g .
+ - run: npm install -g free-programming-books-lint

I'm not expecting anything to go wrong with the current setup, but this isn't ideal.

Why is this valuable (or not)?

I'm hoping this will make it easier for new contributors who don't understand how to check build failures, or can't find the warning in the logs.

Checklist:

• [x] Read our contributing guidelines
• [x] Search for duplicates.

Followup

• Check the output of Travis-CI for linter errors!

[SethFalco]

failed to create review: Resource not accessible by integration

I'm assuming that's because of the repository settings.

~~@eshellman Sorry to pester, could you please peek at the settings: https://github.com/EbookFoundation/free-programming-books/settings/actions~~

~~I'm assuming the one that needs to change is Workflow permissions → Read and write permissions should be selected. This should hopefully give it permission to post reviews.~~

Edit: Ignore the above, this can be done more sensibly by giving the specific permissions in YAML, however that may lead to other problems.

[SethFalco]

Actually… GitHub just tried to run my new workflow in the PR for the new workflow? (Actually that makes sense, but RIP.) Maybe this is a bad idea, actually. 🤔

If GitHub will automatically run workflows for most users, and it will use the workflow configuration from their PR, that would open many opportunities to be malicious. I need to look into this.

I'm currently reviewing the following documents, hopefully I'll have something better soon:

• Automating Dependabot with GitHub Actions (Don't need Dependabot, but sections of it are useful.)
• Keeping your GitHub Actions and workflows secure Part 1: Preventing pwn requests

[eshellman]

Current setting is to allow all actions and "Require approval for first-time contributors who are new to GitHub Only first-time contributors who recently created a GitHub account will require approval to run workflows."

[eshellman]

https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/enabling-features-for-your-repository/managing-github-actions-settings-for-a-repository#allowing-specific-actions-to-run

[eshellman]

If this is not ready yet, we should at least remove Travis and associated documentation before October.

[davorpa]

Conficts with #5587 - commit https://github.com/EbookFoundation/free-programming-books/commit/09bbc1c2d9a5e5f7a257a71546095189d713e27e#diff-d1e75195b3218fdf3e35dbe669c50cb0638f0a506887d47981978a2cfc4554dd

node-version: 14.x -> 16.x

[CleanMachine1]

Whether this will be added is another thing, but regardless over at TLDR, there is a pretty printed message sent https://github.com/tldr-pages/tldr/pull/7883#issuecomment-1072653937 like that.

Adding this helps new contributors easily fix their issues, so adding this would be a useful edition, especially with how many new Github users there are that contribute resources here and often make beginner mistakes.

[CleanMachine1]

Accidentally hit the close and send button

[SethFalco]

I'm thinking about coming back to this one later, but first I'd like https://github.com/vhf/free-programming-books-lint/pull/29 to be resolved. 🤔

I think there was a discussion before about some of the linter repos being transferred to EbookFoundation, is that still a thing?

[davorpa]

I think there was a discussion before about some of the linter repos being transferred to EbookFoundation, is that still a thing?

Yes, you are right. Last word was about if somebody with minimal knowledges takes the control about npm registry account, it said Victor, to be able to make the deploys. In practice I think transfer fpblint would be great!!

[SethFalco]

GitHub made a new feature which might make this cleaner: https://github.blog/2022-05-09-supercharging-github-actions-with-job-summaries/

It doesn't post a comment though, but rather than post a comment here we could use the summary feature and link to that page from here.

[davorpa]

GitHub made a new feature which might make this cleaner: https://github.blog/2022-05-09-supercharging-github-actions-with-job-summaries/

It doesn't post a comment though, but rather than post a comment here we could use the summary feature and link to that page from here.

@LuigiImVector do you see this to be applied to #6914. Use an envvar instead of write in temp file to compose lint report

[LuigiImVector]

@LuigiImVector do you see this to be applied to #6914. Use an envvar instead of write in temp file to compose lint report

I didn't understand, do you mean to save the file name to an envvar?

[davorpa]

@LuigiImVector do you see this to be applied to #6914. Use an envvar instead of write in temp file to compose lint report

I didn't understand, do you mean to save the file name to an envvar?

Sorry for no context. I forgot cite the last Seth's post:

GitHub made a new feature which might make this cleaner: https://github.blog/2022-05-09-supercharging-github-actions-with-job-summaries/

It doesn't post a comment though, but rather than post a comment here we could use the summary feature and link to that page from here.

Doing this I think isn't necesary a sub-workflow

[LuigiImVector]

Doing this I think isn't necesary a sub-workflow

as soon as I have time I will try to implement it and check if it is okay, thanks

[davorpa]

Conficts with #5587 - commit 09bbc1c#diff-d1e75195b3218fdf3e35dbe669c50cb0638f0a506887d47981978a2cfc4554dd

node-version: 14.x -> 16.x

More conflicts with #6696 and #6820 which contains parts with a more recent version (preserve both sides)

[SethFalco]

I'll close this in favor of #6914, where LuigiImVector has made substantially more progress toward this goal.