Use directory arguments on deb job scripts for decoupling (Part Two) #709
Conversation
| @@ -102,7 +102,6 @@ but disabled since the package is blacklisted (or not whitelisted) in the config | |||
| ' ' + os_code_name + | |||
| ' ' + arch + | |||
| ' ' + ' '.join(repository_args) + | |||
| ' --binarydeb-dir $WORKSPACE/binarydeb' + | |||
| ' --dockerfile-dir $WORKSPACE/docker_generating_docker' + | |||
There was a problem hiding this comment.
The same might be the case for this option.
There was a problem hiding this comment.
Looks like this option specifies the output location for this script, so it should stay.
There was a problem hiding this comment.
Oof, actually, its more complicated than that. It's both. I'll see if there is a good way to handle this one.
|
Indeed these were intended to allow decoupling. But if they're not being used cleaning them up probably makes sense. |
@tfoote, would you rather that I implement that functionality instead of removing them? Either way it will need to be a two-stage deployment. Whether the decoupling parameters are present or not, I'd like to do the same thing in the new RPM jobs. |
|
If you want to do it for the RPMs we should actually pass it through here. Otherwise lets just clean it out and we can add it back at some other time if we need the parameterization. |
I plan to make this change when I return from leave. Please keep this ticket open to track the problem until I resolve it. |
Part two of this change. Follows 55f38cd
cottsay
changed the title
@cottsay Friendly ping. What is the status of this ticket? |
Ready for review and merge, I believe. It is technically a breaking change, though. Anyone calling |
@dirk-thomas - there is another breaking change that will follow this (3 parts total). Should we delay merging this until after the release, or are you okay with merging now despite the change in behavior? |
We can wait until part 3 is ready too to minimize the number of breakages. |
|
@cottsay What is the status on this and/or the part 3? |
These values behind these arguments are never referenced in the code. Their intended purpose is fulfilled when specifying the mount directory in the
docker runcall.It is possible that there was some confusion here, and their intended purpose was actually to decouple the Jenkins jobs from the
ros_buildfarmrepository, by specifying where in the container the mountpoint should be expected (instead of where on the host the directory resides). If that is the case, then I'll change this PR to use the arguments more appropriately.Either way, because that decoupling doesn't exist, we'll have to figure out how to deploy this change. Recommendations would be appreciated.