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

Add config to override GitHub's target_commitish values #1061

Closed
mkurz opened this issue Feb 7, 2022 · 10 comments
Closed

Add config to override GitHub's target_commitish values #1061

mkurz opened this issue Feb 7, 2022 · 10 comments

Comments

@mkurz
Copy link
Contributor

mkurz commented Feb 7, 2022

I think I found a solution to various problems people have been reporting.

First, let's imagine following use case (A-G are merge commits, X are tags):

    A  B  C            D  F  G  v2.0.0
    |  |  |            |  |  |    |
----o--o--o-----X------o--o--o----X----> default "main" branch
                |   |               
            v1.0.0  |
 (target_commitish   \
     is "main")       \-------o-----------X----> 1.x branch
                              |           |
                              E         v1.0.1 release

We have a default main branch. At some point, after merging A,B and C we release version v1.0.0. So the target_commitish for this tag is main. Nothing special so far.
Now we continue developing on the main branch and release (or plan to release) a v2.0.0 release. At some point we branch off a 1.x branch from main to release hotfix version v1.0.1 which only contains E.
We configure a release drafter workflow for that 1.x branch to get nice release notes for that v1.0.1 we plan to release, however the problem is, the last release it recognizes is v2.0.0. OK, no problem, lets just set filter-by-commitish: true, so it filters:

const filteredReleases = filterByCommitish
? releases.filter((r) => ref.match(`/${r.target_commitish}$`))
: releases

The problem here is that, like mentioned, the target_commitish for the v1.0.0 tag is main (and not 1.x), that means release drafter does not recognize v1.0.0 as previous release and includes A, B and C (in addition to E) in the release notes for v1.0.1 (but it should contain only E). I could of course change the target_commitish value for v1.0.0 with curl and the GitHub api to 1.x, however this is not a good idea, because now, if we haven't released v2.0.0 yet, the release notes for the default branch will be wrong.

After thinking a while how to solve that problem I think the solution for it (and other problems) is that we need to be able to override the target_commitish values reported by the GitHub api. If we do that for the workflow for the 1.x branch, we could make release drafter believe that the target_commitish for v1.0.0 is 1.x instead of main and everything works perfectly.

My idea is a config like:

commitish_overrides = [
    {
      "tag_name": "v1.0.0",
      "target_commitish": "1.x"
    }
];

The patch should be very simple, it needs to be places just before the above filtering happens, like in this line and will look something like this (not tested yet):

  for (const commitish_override of config.commitish_overrides) {
    let rel = releases.find((r) => commitish_override.tag_name.match(`/${r.tag_name}$`))
    if(rel) {
      rel.target_commitish = commitish_override.target_commitish
    }
  }

commitish_overrides should be an array, so we can override multiple target_commitish, we could even make it support regex, like

commitish_overrides = [
    {
      "tag_name": "v1.*",
      "target_commitish": "1.x"
    }
];

// or change the target_commitsh for all releases:
commitish_overrides = [
    {
      "tag_name": "*",
      "target_commitish": "1.x"
    }
];

This config makes release drafter very flexible, like what you can do as well is:
If you you want to skip a release, like you have a release v1.1.0 now you release v1.2.0, but made a mistake because it contains a regression and want to release v1.3.0 with a fix, but want to have all PR of the 1.2.0 release in the 1.3.0 release notes as well, you can just "exclude" the v1.2.0 release by setting its target_commitish to foo or whatever so it won't be recognised and release drafter will think v1.1.0 was the last release.

commitish_overrides = [
    {
      "tag_name": "v1.2.0",
      "target_commitish": "foo"
    }
];

This would actually work for #422 as well.

(BTW: Everything I wrote here assume filter-by-commitish: true is set)

@jetersen What do you think? After thinking about this for a while I think it makes a lot of sense. If you agree I will provide a pull request including tests.

@mkurz
Copy link
Contributor Author

mkurz commented Feb 8, 2022

This is specially useful if you there are beta and/or RC releases but still want to include all pull requests in the final release.
For example if you released version v1.2.0-RC1,v1.2.0-RC2 and v1.2.0-RC3, when releasing v1.2.0 you could just add:

commitish_overrides = [
    {
      "tag_name": "v1.2.0-RC*",
      "target_commitish": "foo"
    }
];

This way these releases are skipped by release drafter, still including all their merged pull requests in the release notes.

I really like this feature.

@mkurz
Copy link
Contributor Author

mkurz commented Feb 8, 2022

I am adding release drafter to more than 20 repos and I already have problems which this feature definitely would solve.

@mkurz
Copy link
Contributor Author

mkurz commented Feb 8, 2022

@jetersen If you give me green light I will try to come up with a pull request the next days.

@jetersen
Copy link
Member

jetersen commented Feb 8, 2022

@mkurz did you see #1058 ?

@mikeroll
Copy link
Contributor

mikeroll commented Feb 8, 2022

@mkurz IMO the "easy" path of not setting commitish or setting it to some static value like main is unsuitable for any remotely non-trivial setup. The best way to achieve consistent results with different branches is to actually let release-drafter know which branch it's currently running for and make it consider the commits on that branch - this is what #1058 is about:

  • commitish: "main" and it goes back from the HEAD of main, until it finds v1.0.0, the resulting release is for v1.0.0..main
  • commitish: "1.x" and it goes back from the HEAD of 1.x (not from HEAD of main!) until it finds v1.0.0, the resulting release is for v1.0.0..1.x
  • commitish: "${{ github.ref_name }}" and you have the above behaviour for any branch. Or even better "${{ github.sha }}" if that works for you.

No filter-by-commitish needed, which for cases like yours is totally a nasty hack to work around not having the above behaviour (yet).

@jetersen
Copy link
Member

jetersen commented Feb 8, 2022

@mikeroll should we consider removing filter-by-commitish?

@mikeroll
Copy link
Contributor

mikeroll commented Feb 8, 2022

@jetersen I think it still kinda makes sense if someone wants this for whatever reason (commitish: main, @ means target_commitish of the release)? Not entirely sure.

                                                  v2.0.0 (@ main)
                                                   |
----------------X-----------o---------------o------X------ > main
                |           |               |
            v1.0.0 (@ main) |               |
                             \--------X----/ some-branch
                                      |
                                    v1.0.1 (@ some-branch) but I want to pretend it doesn't exist?

@mikeroll
Copy link
Contributor

mikeroll commented Feb 8, 2022

Sorry for adding to the confusion - this has nothing to do with my PR.

The actual problem is that release search and commit search are completely independent, with release search being rather dumb - just sort by version/date and take the latest.
I would first try to bring the two together and connect the commits with the corresponding releases properly (via tags, probably?), rather than trying to explain to it the reality of things via complex configuration.

@mkurz
Copy link
Contributor Author

mkurz commented Feb 9, 2022

I will have a look this week again, I am a bit busy today.

@jetersen
Copy link
Member

jetersen commented Mar 7, 2022

This is fixed by #1065

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

No branches or pull requests

3 participants