Skip to content

add excludeTargetBranch input option #656

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

Closed

Conversation

michaelgwelch
Copy link

Trying to figure out how to test my changes.

  1. How to just test with a live repo
  2. How to add unit tests to your project

Regarding #1: It seems that your version runs instead of mine (even though in my test repo I'm specifically pulling in my version). I assume this is because it downloads and uses your docker image (which I assume has your version of this action in it, or it specifically pulls down your version and runs it).

If you can give me a pointer on how to test this (I don't have a lot of experience writing workflows or custom actions).

Fixes #456 (or will after tested, reviewed, and corrected if necessary)

branch will be linted again. Set this to `true` to avoid that.

TODO: This explanation can be made better when I better understand why `--firstParent` alone didn't resolve this.
To do that I need to understand what `git log <revision range>` is doing.
Copy link
Author

Choose a reason for hiding this comment

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

Likewise, the better explanation can go above as well.

TODO: This explanation can be made better when I better understand why `--firstParent` alone didn't resolve this.
To do that I need to understand what `git log <revision range>` is doing.
default: "false"
required: false
Copy link
Author

Choose a reason for hiding this comment

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

if we understand the root problem better, then perhaps this should default to true much like firstParent.

@@ -78,6 +78,10 @@ function getHistoryCommits(from, to) {
options.firstParent = true
}

if (getInput('excludeTargetBranch') === 'true') {
options[`^${GITHUB_BASE_REF}`] = true
}
Copy link
Author

Choose a reason for hiding this comment

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

Not 100% certain I have the correct variable here since this code has literally never run. (See comment in description about "how to test")

@michaelgwelch
Copy link
Author

Based on my discover and later comment #456 (comment) , I think this PR should be closed and instead I could submit a PR where we use git show and a list of shas to get the commit messages.

@wagoid wagoid closed this Apr 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

What range of commits are looked at?
2 participants