Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci(workflow): post build result as comment #5564

Closed
wants to merge 2 commits into from

Conversation

SethFalco
Copy link
Sponsor Member

@SethFalco SethFalco commented Sep 28, 2021

What does this PR do?

Improve repo

Description

  • 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.

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:

Followup

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

@SethFalco
Copy link
Sponsor Member Author

SethFalco commented Sep 28, 2021

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:

@SethFalco SethFalco marked this pull request as draft September 28, 2021 08:33
@SethFalco SethFalco force-pushed the post-failures branch 2 times, most recently from ece0dd8 to e9f7dff Compare September 28, 2021 10:53
@eshellman
Copy link
Collaborator

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."

@davorpa davorpa added the New Feature New feature / enhancement / translation... label Sep 28, 2021
@eshellman
Copy link
Collaborator

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

@davorpa davorpa mentioned this pull request Oct 3, 2021
5 tasks
@davorpa davorpa added the conflicts Conflict(s) need to be resolved label Nov 5, 2021
@davorpa

This comment was marked as outdated.

@davorpa davorpa changed the title workflow: post build result as comment ci(workflow): post build result as comment Dec 31, 2021
davorpa added a commit to davorpa/free-programming-books that referenced this pull request Feb 5, 2022
@davorpa davorpa added the 🤖 automation Automated tasks done by workflows or bots label Mar 5, 2022
eshellman pushed a commit that referenced this pull request Mar 16, 2022
… of files (#6696)

* config: addopt current file formatting as yml & markdown parametrization

Cherry picked from #5564: e9f7dff

* add config properties for JSON files

* extend config to all markdown file extensions

* sort global properties

* add mapping for python files

* add windows batch files mapping

* sort mapping entries

* add makefile mapping properties

* add file description / summary

* add poweshell mappings

* Comment each section

* disable linebreaks on supported editors

* Apply some suggestions

* clean deadcode

- simplify properties already defined in global section
- remove not used future patterns
@CleanMachine1
Copy link
Member

Whether this will be added is another thing, but regardless over at TLDR, there is a pretty printed message sent tldr-pages/tldr#7883 (comment) 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
Copy link
Member

CleanMachine1 commented Apr 2, 2022

Accidentally hit the close and send button

@SethFalco
Copy link
Sponsor Member Author

I'm thinking about coming back to this one later, but first I'd like vhf/free-programming-books-lint#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
Copy link
Member

davorpa commented Apr 4, 2022

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
Copy link
Sponsor Member Author

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 davorpa linked an issue Jul 13, 2022 that may be closed by this pull request
@davorpa
Copy link
Member

davorpa commented Aug 10, 2022

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
Copy link
Member

@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
Copy link
Member

davorpa commented Aug 11, 2022

@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
Copy link
Member

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
Copy link
Member

davorpa commented Aug 13, 2022

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)

@github-actions github-actions bot removed the conflicts Conflict(s) need to be resolved label Oct 20, 2023
@SethFalco SethFalco force-pushed the post-failures branch 2 times, most recently from 545b485 to 39a648b Compare October 20, 2023 14:32
@SethFalco
Copy link
Sponsor Member Author

SethFalco commented Oct 20, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤖 automation Automated tasks done by workflows or bots New Feature New feature / enhancement / translation...
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Linting Errors as PR Comments
5 participants