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

makefile: add check-pr rule #3215

Merged
merged 1 commit into from
Jun 12, 2023
Merged

Conversation

geyslan
Copy link
Member

@geyslan geyslan commented Jun 7, 2023

1. Explain what the PR does

9021720 makefile: add check-pr rule (2023/jun/07) Geyslan Gregório <geyslan@gmail.com>

This rule will run all code checks (not tests) and output a formatted
summary of all commits that are not yet in the main branch to be used
for pull requests head comments.

2. Explain how to test it

From your working branch, after commits created, run make check-pr.

3. Other comments

Chore.

@rafaeldtinoco
Copy link
Contributor

Hum, its best if we change "resume" (meaning returning from interruption) to something that describes "summary". make pr-summary ? Or something simpler, if you prefer.

@rafaeldtinoco
Copy link
Contributor

Also, please keep the Author format as "Name email@domain" please. Anything git related usually use that as a reference to authors.

Other than that, I like the summary idea, we have to advertise in the github template as well .

@geyslan
Copy link
Member Author

geyslan commented Jun 7, 2023

Hum, its best if we change "resume" (meaning returning from interruption) to something that describes "summary". make pr-summary ? Or something simpler, if you prefer.

make before-push?
make check-push?

@geyslan geyslan changed the title makefile: add pr-resume rule makefile: add check-pr rule Jun 7, 2023
@geyslan
Copy link
Member Author

geyslan commented Jun 7, 2023

make before-push? make check-push?

check-pr for the win.

@geyslan geyslan modified the milestone: v0.16.0 Jun 7, 2023
Copy link
Contributor

@rafaeldtinoco rafaeldtinoco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@rafaeldtinoco
Copy link
Contributor

ACTUALLY... thinking straight now.. WDYT about removing the ========== ? I know you fixed a 80 cols to the "====" line, but sometimes terminals aren't fully 80 cols, and it brakes terminal output. I'd rather have **** SOMETHING **** or something else, but not a header that obligates to have a specific number of columns in the terminal, WDYT ?

@geyslan
Copy link
Member Author

geyslan commented Jun 12, 2023

image

This rule will run all code checks (not tests) and output a formatted
summary of all commits that are not yet in the main branch to be used
for pull requests head comments.
@rafaeldtinoco
Copy link
Contributor

Yep, thanks for the change/fix.

@rafaeldtinoco rafaeldtinoco merged commit 2d21743 into aquasecurity:main Jun 12, 2023
2 checks passed
@geyslan geyslan deleted the pr-resume branch July 31, 2023 22:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants