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

Override calculated version, tag and name with input #363

Merged
merged 11 commits into from
Jan 22, 2020

Conversation

pdcmoreira
Copy link
Contributor

Allow overriding the resulting version, tag and name throught input options.
This is useful for workflows scoped by branches (#359) that need to transition a version from one branch to another - GitFlow, for example.

@TimonVS
Copy link
Member

TimonVS commented Jan 4, 2020

Thanks for the PR @pdcmoreira. This is an awesome addition. I'm going to give it a test run!

Copy link
Member

@TimonVS TimonVS left a comment

Choose a reason for hiding this comment

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

Tested it for a bit. The functionality is great, I have a few suggestions for improvements :)

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
lib/versions.js Outdated Show resolved Hide resolved
@TimonVS TimonVS added the type: feature New feature or request label Jan 6, 2020
@pdcmoreira
Copy link
Contributor Author

Rebased onto master and separated the manual input version from the calculated ones.
@TimonVS Please check if that's what you had in mind.

@pdcmoreira
Copy link
Contributor Author

Rebased onto master and fixed a bug with the mocked env that was leaking into other tests.

Copy link
Member

@jetersen jetersen left a comment

Choose a reason for hiding this comment

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

LGTM

@pdcmoreira
Copy link
Contributor Author

Rebased onto master.
By having the input version and the calculated version separated, I noticed that there was no way to create a generic template that would print the input version if available or the calculated one otherwise, so I've added a new $RESOLVED_VERSION variable that does just that.

@TimonVS
Copy link
Member

TimonVS commented Jan 21, 2020

$RESOLVED_VERSION is a great addition @pdcmoreira! I'm testing it one last time and then I'll merge and release it.

@TimonVS
Copy link
Member

TimonVS commented Jan 21, 2020

After testing this I noticed that this only works for newly created drafts. I think we should also propagate this through draft updates. This is technically a breaking change, but I would consider it a bug if we don't update the draft, so I think a minor version in this case would still be fine.

@TimonVS
Copy link
Member

TimonVS commented Jan 21, 2020

@pdcmoreira @Casz what do you think about propagating the release's name and tag changes in draft updates too? As mentioned in my previous comment, it's technically a breaking change, but I think it can be considered a bug fix because currently the release name and tag for draft releases won't update even though the input version might have changed. But, people might rely on this functionality to update the title or tag of a release manually without having Release Drafter override it with a draft update. What are your thoughts on this?

I have a reproduction for this issue here: https://github.com/TimonVS/rd-override-version. You can test this by having Release Drafter generate a draft first and then change the input version here: https://github.com/TimonVS/rd-override-version/blob/master/.github/workflows/release-drafter.yml#L16. You'll see that the version in the body is updated but that the version in the title and tag stay the same.

@pdcmoreira I rebased your PR btw so that I could test with the latest changes.

@jetersen
Copy link
Member

I honestly thought it was a bug but not one I cared to report 🐛😅 Yes I think it is a bug.

@pdcmoreira
Copy link
Contributor Author

True, I think it should be considered a bug, because if the calculated version changes, it stops being semver compliant.
But it's also true that it might stop fitting some use cases.
My suggestion would be to "fix" the "bug" and add an option to ignore version re-calculation on draft updates.

@TimonVS
Copy link
Member

TimonVS commented Jan 22, 2020

Thanks for your input! I'll create a fix for it, once that's done we'll release this. I've temporarily removed the documentation in this PR so that I can merge it now without having incorrect documentation on the master branch.

@TimonVS TimonVS merged commit 36d7384 into release-drafter:master Jan 22, 2020
@pdcmoreira pdcmoreira deleted the manually-set-tag branch January 22, 2020 11:05
@jetersen
Copy link
Member

jetersen commented May 26, 2020

@pdcmoreira I had issues with failing tests due to mockedEnv on my Windows box.
Seems to be related only to the INPUT_CONFIG-NAME mockedEnv 😓

23:01:54.379Z  WARN probot: toolmantim/release-drafter-test-project: Invalid config file
  HttpError: request to https://api.github.com/repos/toolmantim/release-drafter-test-project/contents/.github/config-name-input.yml failed, reason: Nock: No match for request {
    "method": "GET",
    "url": "https://api.github.com/repos/toolmantim/release-drafter-test-project/contents/.github/config-name-input.yml",
    "headers": {
      "accept": [
        "application/vnd.github.v3+json"
      ],
      "user-agent": [
        "octokit.js/16.43.1 Node.js/14.2.0 (Windows 10; x64)"
      ],
      "authorization": [
        "token test"
      ],
      "accept-encoding": [
        "gzip,deflate"
      ],
      "connection": [
        "close"
      ]
    }
  }

@jetersen
Copy link
Member

The mockedEnv issue disappeared.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants