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 TargetCommitish to CreateReleaseParams #2367

Merged
merged 1 commit into from Aug 17, 2019

Conversation

@nikolamilekic
Copy link
Contributor

commented Aug 7, 2019

Description

Added TargetCommitish parameter to the CreateReleaseParams record. This parameter is passed to Octokit's 'NewRelease', and allows for the creation of releases from arbitrary commits when the release tag does not exist yet.

@nikolamilekic nikolamilekic marked this pull request as ready for review Aug 7, 2019
@matthid

This comment has been minimized.

Copy link
Collaborator

commented Aug 7, 2019

Thanks! Looks good to me. Any idea if we could somehow test this feature? (Is there some testing instance for the github api, for example?)

@nikolamilekic

This comment has been minimized.

Copy link
Contributor Author

commented Aug 7, 2019

Thanks! Looks good to me. Any idea if we could somehow test this feature? (Is there some testing instance for the github api, for example?)

I don't know. :(

I tested it by copy/pasting the whole createRelease function directly into my build script.
Here's it is: https://github.com/nikolamilekic/SaltShaker/blob/master/build.fsx.

What kind of tests would you like to see? Examples of tests of other API calls would be helpful as I'm unfamiliar with the codebase.

@matthid

This comment has been minimized.

Copy link
Collaborator

commented Aug 7, 2019

Just asking I don't have anything particular in mind. Just assumed you are more familiar with the github api.

What kind of tests would you like to see?

Ideally something which verifies that this works without the need for me to test manually ;)

Examples of tests of other API calls would be helpful as I'm unfamiliar with the codebase.

Sadly, I don't think we have any.

In any case this looks good to me, so I guess I'll just merge as is...

@matthid
matthid approved these changes Aug 7, 2019
@nikolamilekic

This comment has been minimized.

Copy link
Contributor Author

commented Aug 7, 2019

Just assumed you are more familiar with the github api.

Unfortunately, I'm not. I just assumed there's a way for me to create a release without a tag. After not finding a parameter in CreateReleaseParams I looked under the hood to find the Octokit's NewRelease and there it was. Tested it; it worked; thought it might be useful to others and so here we are.

In any case this looks good to me, so I guess I'll just merge as is...

Thanks!

@matthid matthid merged commit 2b47bb0 into fsharp:release/next Aug 17, 2019
3 checks passed
3 checks passed
FAKE-CI #1177 succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@matthid matthid referenced this pull request Aug 17, 2019
@nikolamilekic nikolamilekic deleted the nikolamilekic:TargetCommitish branch Aug 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.