Conversation
d10e214 to
83554a6
Compare
SMoraisAnsys
left a comment
There was a problem hiding this comment.
@jorgepiloto I think you only have to change the file name to have your changes working
Regarding the changes, I don't have a strong opinion and am fine with the proposed changes.
RobPasMue
left a comment
There was a problem hiding this comment.
We should also check that other actions like the GitHub release are not impacted by this change. I'm not sure what naming convention it was expecting for wheelhouse artifacts.
Good point. So far, I checked the release ones. They use a glob pattern like |
moe-ad
left a comment
There was a problem hiding this comment.
Shell substitutions will not work in this situation. It should be interpolated directly. This needs to be repeated for other cases as well.
Co-authored-by: Maxime Rey <87315832+MaxJPRey@users.noreply.github.com> Co-authored-by: Muhammed Adedigba <68085496+moe-ad@users.noreply.github.com>
Co-authored-by: Muhammed Adedigba <68085496+moe-ad@users.noreply.github.com>
|
@SMoraisAnsys @RobPasMue |
RobPasMue
left a comment
There was a problem hiding this comment.
Looks good to me I think - let's see if it has some unexpected impact in other places we are not aware currently
SMoraisAnsys
left a comment
There was a problem hiding this comment.
LGTM, I checked the repo and couldn't find a place where those changes could be a problem.
There was a problem hiding this comment.
@moe-ad Could you perform a test from an external repo to ensure that nothing is missed ? This would require to point to the PR's branch. Still thinking that this is fine but calling a request for changes to ensure that someone does take the time to test before merging.
There is no convention for wheelhouses as opposite to Python distribution artifacts. Knowing that these last impose underscores in the name of the projects but use hyphens for separating metadata, I decided to follow the same approach with the wheelhouse. Fixes #681.