marcoabreu commented on a change in pull request #18222:
URL: https://github.com/apache/incubator-mxnet/pull/18222#discussion_r419073017
##########
File path: cd/utils/artifact_repository.py
##########
@@ -396,7 +396,7 @@ def get_s3_key_prefix(args: argparse.Namespace, subdir: str
= '') -> str:
:param subdir: An optional subdirectory in which to store the files.
Post-pended to the end of the prefix.
:return: A string containing the S3 key prefix to be used to uploading and
downloading files to the artifact repository
"""
- prefix = "{git_sha}/{libtype}/{os}/{variant}/".format(**vars(args))
+ prefix = "{git_sha}/{libtype}/{variant}/".format(**vars(args))
Review comment:
Creating a binary on unix, windows, arm etc will result in different
binaries which have to be differentiated. It might be a valid point that the os
should not be the differentiator, but it should be properly addressed (or
shown) that these mentioned cases are properly considered.
When removing something, it's up to the PR creator to prove that it will not
cause any harm and that the initial design decision are not broken - or
otherwise explicitly state it and make an alternative proposal. "fix" is not a
valid argument or proposal.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
