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

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 August 7, 2019 15:23
@matthid
Copy link
Member

matthid 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
Copy link
Contributor Author

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
Copy link
Member

matthid 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...

@nikolamilekic
Copy link
Contributor Author

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 fsprojects:release/next Aug 17, 2019
@matthid matthid mentioned this pull request Aug 17, 2019
@nikolamilekic nikolamilekic deleted the TargetCommitish branch August 22, 2019 04:56
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.

2 participants