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

chore(build): add LOGFROM flag to check-pr rule #3348

Merged
merged 1 commit into from
Jul 28, 2023

Conversation

geyslan
Copy link
Member

@geyslan geyslan commented Jul 27, 2023

1. Explain what the PR does

cf71376 chore(build): add LOGFROM flag to check-pr rule

commit cf71376

With this flag, the git log output is filtered to only include commits
from the specified commit to the current HEAD.

make check-pr                # from main (default)
make check-pr LOGFROM=v0.1.0 # from tag/branch
make check-pr LOGFROM=HEAD~1 # from commit
make check-pr LOGFROM=f3a4b2 # from commit

This also changes check-pr output format and adds information for using
its output in the PULL_REQUEST_TEMPLATE.md file.

2. Explain how to test it

See commit above.

3. Other comments

@rafaeldtinoco
Copy link
Contributor

rafaeldtinoco commented Jul 28, 2023

can we change a bit the format from "check-pr"? I'm not using it because I don't like the format. I use it to check everything needed, but I wont copy and paste the results.

2 things that bothers me:

  1. Format:

93a5329 chore(build): add LOGFROM flag to check-pr rule (2023/jul/27) Geyslan Gregório geyslan@gmail.com

I believe we can make:

<hash> description

  1. The "more" or "less" my pager does when make check-pr finishes.

Can we make those adjustments in this PR ?

With this flag, the git log output is filtered to only include commits
from the specified commit to the current HEAD.

make check-pr                # from main (default)
make check-pr LOGFROM=v0.1.0 # from tag/branch
make check-pr LOGFROM=HEAD~1 # from commit
make check-pr LOGFROM=f3a4b2 # from commit

This also changes check-pr output format and adds information for using
its output in the PULL_REQUEST_TEMPLATE.md file.
@geyslan geyslan merged commit 8e2086a into aquasecurity:main Jul 28, 2023
2 checks passed
@rafaeldtinoco
Copy link
Contributor

image

Feedback from this change:

I think the last list of commits should include the subject as well, to avoid empty stuff when the commit logs have a single subject line with no other lines.

Also, I still have a pager automatically called for the last part (it would be good not to have it so I can copy and paste all the output at once).

@rafaeldtinoco
Copy link
Contributor

@geyslan FYI

@geyslan
Copy link
Member Author

geyslan commented Jul 31, 2023

Ok, raising other PR with that change.

xclip -selection clipboard can be used but it would demand to check the tool and possibly override clipboard content. So, related to that, I'll let it as it is.

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