Skip to content

Fix #37 change version_tuple to string #38

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

Merged
merged 2 commits into from
Oct 14, 2022
Merged

Conversation

shenxianpeng
Copy link
Collaborator

@shenxianpeng shenxianpeng commented Oct 13, 2022

Fixed #37

@shenxianpeng shenxianpeng added the bug Something isn't working label Oct 13, 2022
@shenxianpeng shenxianpeng requested a review from 2bndy5 October 13, 2022 10:01
Copy link
Contributor

@2bndy5 2bndy5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer the tuple be integers, but I guess if python can compare a tuple of strings it should be ok as long as we're looking for an exact matching version number.

Co-authored-by: Brendan <2bndy5@gmail.com>
@codecov-commenter
Copy link

codecov-commenter commented Oct 13, 2022

Codecov Report

Merging #38 (b8edfaf) into main (ce41d4f) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main      #38   +/-   ##
=======================================
  Coverage   87.71%   87.71%           
=======================================
  Files           4        4           
  Lines         171      171           
=======================================
  Hits          150      150           
  Misses         21       21           
Impacted Files Coverage Δ
clang_tools/install.py 82.69% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@shenxianpeng
Copy link
Collaborator Author

shenxianpeng commented Oct 13, 2022

ver_num.groups(0)[0].decode(encoding="utf-8").split(".") this is a tuple of strings so I also change the version number to string. Compare strings in python should be ok

@2bndy5
Copy link
Contributor

2bndy5 commented Oct 13, 2022

Typically, we compare tuples of integers in python when determining if a version number is greater or less.

>>> (1, 0, 0) > (0, 1, 0)  # v1.0.0 > v0.1.0
True

Like I said, comparing a tuple of strings should be adequate for matching an exact version number.

@shenxianpeng shenxianpeng merged commit 952097d into main Oct 14, 2022
@shenxianpeng shenxianpeng deleted the fix-version_tuple branch October 14, 2022 00:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

clang-tools still download binary even native binary exist
3 participants