include option for nested dist #26
Conversation
TomVasile
changed the title
.drone.yml
Outdated
| image: golang:1.11-alpine | ||
| commands: | ||
| - apk --no-cache add -U python3 git | ||
| - apk --no-cache add -U python3 git python3-dev gcc libc-dev libffi-dev openssl-dev |
There was a problem hiding this comment.
These changes don't seem to be related to this PR, just like changes below in Dockerfiles
There was a problem hiding this comment.
@lafriks You can't build the project without these changes currently. See CI failures without them: https://cloud.drone.io/drone-plugins/drone-pypi/53/1/4
|
@tboerger could be more help in this as I'm not familiar with this plugin that much |
|
Also I just noticed this, these dependency changes were approved already and were never merged. |
|
Can you revise? |
I can if you merge the other PR, the project does not build without those changes. Or close that PR and have them just merge in with mine. Whichever you guys prefer. |
|
@lafriks Please let me know what you would like me to do here. |
plugin.go
Outdated
| args = append(args, "--password") | ||
| args = append(args, p.Password) | ||
| args = append(args, "dist/*") | ||
| args = append(args, filepath.Join(filepath.Dir(p.DistDir), "/*")) |
There was a problem hiding this comment.
you can simplify to this:
filepath.Join(p.DistDir, "/*")
There was a problem hiding this comment.
@bradrydzewski Thanks! Pushed this change.
|
Please rebase as the other PR have been merged. |
@tboerger Done |
|
quick question ... wondering if we could simplify this a bit further? cli.StringFlag{
Name: "dist_dir",
Usage: "used when distribution directory is not in build root",
- Value: "dist/",
+ Value: "dist/*",
EnvVar: "PLUGIN_DIST_DIR",
},-args = append(args, filepath.Join(p.DistDir, "/*"))
+args = append(args, p.DistDir) |
So the reason I did it that way was so that the end user didn't have to specify I guess I could change it to be a "path_to_dist_file(s)" envvar. What are your thoughts? I'm kind of indifferent as both fit my use case. |
|
I think the below patch is preference since it is more consistent for how we handle this with other plugins. cli.StringFlag{
Name: "dist_dir",
Usage: "used when distribution directory is not in build root",
- Value: "dist/",
+ Value: "dist/*",
EnvVar: "PLUGIN_DIST_DIR",
},-args = append(args, filepath.Join(p.DistDir, "/*"))
+args = append(args, p.DistDir) |
|
@bradrydzewski Sounds good, will pick this back up shortly. |
Fixes issue where dist directory is assumed to be in the base directory. With this change, you can specify if that directory exists elsewhere.
Edit: Also fixed build dependencies since I was unable to build the project in it's current state without them.