-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
✨ ENH: Auto-detecting the target #45
Conversation
635ed24
to
93312f1
Compare
github_activity/github_activity.py
Outdated
response = requests.get(f"https://api.github.com/repos/{org}/{repo}/git/refs/tags") | ||
response.raise_for_status() | ||
tags = response.json() | ||
latest_tag = list(tags)[-1] | ||
return latest_tag["ref"].split("/tags/")[-1] | ||
out = run("git describe --tags".split(), stdout=PIPE) | ||
tag = out.stdout.decode().rsplit('-', 2)[0] | ||
return tag |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will only be the same if the local repo is updated, but I think it's fine and may make sense to check using the local repo's latest tag rather than remote repos anyhow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah the other challenge I realized is that using the GitHub API here doesn't sort by latest updated, it sorts alpha-numerically on the tag name. So this will make it more likely we get the latest tag
if "upstream" in remotes: | ||
ref = remotes["upstream"] | ||
elif "origin" in remotes: | ||
ref = remotes["origin"] |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
I'm very happy about this feature!! ❤️ 🎉 |
Co-authored-by: Erik Sundell <erik.i.sundell@gmail.com>
ok I think that this is ready to go! I also fixed up our test CI/CD so it'll run properly now. @consideRatio lemme know if you have any other suggestions! |
All systems go! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried locally, had some issues, will report back.
@choldgraf I've created a PR to this PR resolving the issue I experienced, related to having a SSH based git remote -v
reference.
ok - got the tests working! I think we actually fixed a bug in the regression tests that I didn't catch before. Gonna merge this one! |
closes #21