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
feat(generics): allow upload of build assets as generic packages #174 #343
feat(generics): allow upload of build assets as generic packages #174 #343
Conversation
8f5fbb8
to
d6716c1
Compare
@JonasSchubert Could you check if it makes sense to pull the changes from #322 in this MR? It seems to touch the very same code paths. |
Will check |
d6716c1
to
7099972
Compare
I would tackle this in a separate PR. I checked the code and in my opinion it needs major adjustments regarding assets handling. |
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.
Again just a small documentation suggestion.
I would wait for #322 and merge afterwards. |
Fine for me! Ping me again in case you change your mind :) |
e404962
to
2050fa9
Compare
any update on this? |
@fgreinacher @JonasSchubert #322 has been merged. Can you please merge this PR? |
Happy to merge once conflicts are resolved. @JonasSchubert are you still willing to work on this? |
Sorry everyone! I lost track of some stuff... |
fa5680c
to
cb7ce48
Compare
@fgreinacher Rebased to v10 and fixed lint errors. Should be fine now. |
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.
LGTM, let's get this in finally :)
🎉 This PR is included in version 10.1.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Thanks for the work to all, and for merging this. I've tried this feature and I guess there are two bugs:
The url should be like this:
This code should fix: I got this error creating labels like this: "MyApp x86 (.dmg)"
After few minutes the package is no longer available, loading the package url I got this response:
I guess that this can fix the bug: I can do the PRs if needed |
Hi @mfoti and thanks for your report.
As the README describes, the
This is a little bit more tricky then For me it looks like you use Google Storage to store some assets and packages and this provider returns an URL with an expiration date. Not sure whether this is for the URL only or also for the file. BTW the currently used URL is the one provided by the API call response according to the docs. But:
With the current approach we have direct access to the uploaded file without worrying about this drawback, which we might loose using the Gitlab URL. @mfoti what are your thoughts on this? |
I guess defaults to the filename, but allow to put a string as package description, as in my case: "MacOS x64 (.dmg)" BTW I can confirm that using the filename as label works:
But if you use a different label it doesn't
I guess to avoid errors (release is correctly generated with a label different of the filename using the default target) we can just ignore the label in the code and use the filename here: https://github.com/semantic-release/gitlab/blob/master/lib/publish.js#L88 IMO is better to give the ability to users to use their own label. BTW This is not a blocking behavior, when the target is to
yep man, it is lol 🤣
yes, I already tested it and it works :) I have a fork of the project with this fix and I can confirm resolve the issue.
Yes you are right, but I guess is not well documented, I'll try to find the code on gitlab repo to check better the behavior. What can I say is that my builds are greater the 200mb, probably gitlab uses google storage if the size of the package is bigger than a certain value. I don't know. But the url provided in filename is broken in my case, and this is a blocking bug that doesn't allow me to use this kind of release
To be honest I don't understand which is this case, let's assume I'll build two different packages with the same name, for example:
And try to publish them. In this case I will have an url like: That of course will store the last published file to that path. At the end you will always get the most recent (last published) package downloading the content from the package path The main problem is that it doesn't works for me as is now, I have to use my fork. |
I guess there is another feature that can be used too, the package type. At the moment it is hardcoded:
This type value should be taken from the configuration, this should fix:
The final effect, without hardcoding the type, will be this: |
@mfoti You already created a fork with a fix on your side. Feel free to create the PR ;) |
I should squash, but is a really easy fix: #504, this fixes only the main "bug", but not the package type and the filename/label |
IMO the current implementation is correct here. Generic packages only should have the type |
I think this is a create idea. Would you like to add it to your PR too? |
Gitlab allows to add generic packages as assets attached to a release. #174 This is a suggestion to define a list of generics to be published into the generics repository of a project and then to be used as assets for the release.