ndimiduk commented on pull request #2015:
URL: https://github.com/apache/hbase/pull/2015#issuecomment-658464588
> I'm fine with this change as is, but longer term there's a code smell
around the ASF_REPO variable.
>
> it gets defined in `get_release_info` but does not appear to be
intentionally persisted or exported from there. so I suspect the
`check_for_tag` method only works there and I'm not sure we are gaining over
using `git ls-remote` directly
I could replicate the defaulting logic from `get_release_info`,
```
if [[ -z "${ASF_REPO}" ]]; then
ASF_REPO="https://gitbox.apache.org/repos/asf/${PROJECT}.git";
fi
```
into `check_for_tag`. That would make it safe for use outside of that
context.
I don't follow this comment -- the proposed patch _does_ use `git
ls-remote`. Do you mean to say, use `git ls-remote` _without_ specifying the
remote repository? The man page says `<repository>` is optional, so we could
omit it, though I'm not clear what the default remote is at this point. I like
keeping things explicit if we can.
----------------------------------------------------------------
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]
