feat: Add support for providing a base branch and return diff of commits #155
Conversation
|
|
|
Hi @csyme-cmc, thanks for raising this, apologies it has taken us a while to get to taking a look. Would you be able to give us an idea of the use case you are looking to handle here please? For example is it to get the commits for a PR, or maybe between release branches? Thanks. |
|
Hi @geofflamrock, |
Thanks, we'll discuss this internally and come back to you on the changes. I'm on leave next week so likely will get back to this the week after when I'm back on board. Thanks again for raising the PR and for your patience whilst we take a look at it. |
|
@geofflamrock any update on this? |
There was a problem hiding this comment.
Hi @csyme-cmc, thanks again for taking the time to raise this PR. We've taken a look and think the approach you've taken here seems reasonable and would be happy to work with you to get it to the point where it can be merged in.
I've left a few comments inline around some improvements that we think should be made, in particular adding some tests for the new functionality as well as examples of usage.
| @@ -21,6 +21,8 @@ export interface InputParameters { | |||
| version: string | |||
| branch?: string | |||
| overwriteMode: OverwriteMode | |||
| baseBranch?: string | |||
| githubToken: string | |||
There was a problem hiding this comment.
This property looks like it's only used in conjunction with baseBranch, I think it probably makes sense for this to be optional.
| @@ -52,6 +52,8 @@ steps: | |||
| | `server` | The instance URL hosting Octopus Deploy (i.e. "https://octopus.example.com/"). The instance URL is required, but you may also use the OCTOPUS_URL environment variable. | | |||
| | `api_key` | The API key used to access Octopus Deploy. An API key is required, but you may also use the OCTOPUS_API_KEY environment variable. It is strongly recommended that this value retrieved from a GitHub secret. | | |||
| | `space` | The name of a space within which this command will be executed. The space name is required, but you may also use the OCTOPUS_SPACE environment variable. | | |||
| | `base_branch` | The base branch to compare the commits to. If omitted only the last push commit will be used. | | |||
There was a problem hiding this comment.
I think it might be helpful to add some examples up above of how to use this for some of the use cases it unlocks.
| const baseBranch = parameters.baseBranch | ||
| let commits: IOctopusBuildInformationCommit[] | ||
|
|
||
| if (baseBranch) { |
There was a problem hiding this comment.
I think it would be good to add some tests for this functionality, there are a couple of existing tests in __tests__/main.test.ts file that you could look to add to for this.
This enhancement introduces the capability to specify a base branch and utilize GitHub's compare commits feature. This enables the generation of a commit list associated with the package.
It's important to note that this modification is non-breaking, activating only when both the
base_branchandgithub_tokeninputs are provided.