-
Notifications
You must be signed in to change notification settings - Fork 11
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
#13 - Allow send files after the tag/release be created. #14
Closed
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The second parameter is named
currentTag
, but the instead the version is passed. The question is, if version and/or tag are necessary at all. The sample code in README.md could also be written asAlternatively, the closure could be named
releaseAssets
(plural) instead, without parameters, expected to return a List of files. And instead of callingreleaseAsset(release.assets())
, it would bewith
But that is less powerful. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or we could define a member releaseAssets of type FileCollection and create a method
releaseAssets(Object)
:The above would require some more changes, because the ChangelogService does not know the project yet. But it would allow the user of the plugin to just write
because project.files can handle Task instances and automatically uses the task outputs (https://docs.gradle.org/current/dsl/org.gradle.api.Project.html#org.gradle.api.Project:files(java.lang.Object[]))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The main reason, why I am in favor of the last suggestion: the usage of jcabi-github should be an implementation-detail. That's the reason, why the setter and getter of SemanticReleaseChangeLogService.github were marked as deprecated and will be removed in 2.0.0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to send other kind of files too (.war, .exe), and a way to rename it. About of the file type the method URLConnection.guessContentTypeFromName() don't have all kind of files too.
We can have a default implementation but I don't know the best way to do it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right. URLConnection.guessContentTypeFromName() doesn't know a lot of file types. E.g. interestingly it does not support jar files.
Do you know, what github uses this information for? I currently have
application/octet-stream
as fallback.The possibility to rename an asset or give a description -- which does not seem to be possible with jcabi-github, but is part of the github-api (https://developer.github.com/v3/repos/releases/#upload-a-release-asset) -- is a valid one. With my changes, the only way to achive giving a different name would be to create a copy-task and use that copy instead of the original file.
Thinking about this: by calling project.files() inside the plugin code masks the fact, that this only works for tasks that produce a single file (like Jar and Zip), but not for other tasks (like Copy. By removing this code in the ChangeLogService again and changing the method to
void releaseAssets(FileCollection)
the code of ChangeLogService would become more clear again, but the user of the plugin would have to write a little more code.Or there could be some overloaded methods
Asset releaseAsset(AbstractArchiveTask)
,Asset releaseAsset(File)
and a new ClassAsset
with methodsAsset contentType(String)
,Asset name(String)
andAsset label(String)
. This would allow to writeBy changing the methods in the ChangeLogService to
Asset releaseAsset(Map params = [:], AbstractArchiveTask task)
, the user could writereleaseAsset sourcesJar, contentType: 'application/zip', name: 'thesources.jar', label: 'the sources jar'
in their build file.