Skip to content
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

Automatically detect implementation languages on GitHub #1203

Merged
merged 2 commits into from Aug 6, 2018

Conversation

david-a-wheeler
Copy link
Collaborator

If a project is created, and its repo is on GitHub, use the
GitHub API to discover the implementation languages in use
and fill in that information. This required updating
several vcr_cassettes.

Signed-off-by: David A. Wheeler dwheeler@dwheeler.com

If a project is created, and its repo is on GitHub, use the
GitHub API to discover the implementation languages in use
and fill in that information.  This required updating
several vcr_cassettes.

Signed-off-by: David A. Wheeler <dwheeler@dwheeler.com>
@codecov
Copy link

codecov bot commented Aug 4, 2018

Codecov Report

Merging #1203 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #1203   +/-   ##
======================================
  Coverage     100%    100%           
======================================
  Files          47      47           
  Lines        1684    1693    +9     
======================================
+ Hits         1684    1693    +9
Impacted Files Coverage Δ
app/lib/github_basic_detective.rb 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2394343...3d58335. Read the comment docs.

:"C#" => 29_834,
HTML: 2_290,
Ruby: 1_359
)
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems like a bad thing to build into a test, since the values will change the next time the cassette is re-recorded, right?

Could you instead run on a known value that won't change, like the values of a specific release: https://github.com/kubernetes/kubernetes/tree/release-1.0

Copy link

Choose a reason for hiding this comment

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

Oh wait - these values will not change when the cassette is re-recorded. The hash value here is sent to a specific method which reformats them from a string - it doesn't retrieve anything from GitHub. The JSON values here were originally from assimilation, but I just used those because it's a real-world complicated list. So it turns out that it's not a problem here!

@dankohn
Copy link
Contributor

dankohn commented Aug 4, 2018

LGTM if you address my one comment.

@david-a-wheeler
Copy link
Collaborator Author

I'm not sure fixing the version is better. I'd like the test to be like the real thing as much as possible. I think it is highly unlikely for these values to change anyway; if they do, we can change the values to match. Also, the mismatch can only occur when we recompute cassettes, which is rare.

@david-a-wheeler
Copy link
Collaborator Author

Fixing the version to analyze does not relly fix the results anyway; this is a service request, and GitHub sometimes updates the heuristics used in their analysis to determine languages.

@dankohn
Copy link
Contributor

dankohn commented Aug 4, 2018

Simplest would be for ciitest to fork a repo (like the best practices one) and then not change it. That should be relatively stable.

@david-a-wheeler
Copy link
Collaborator Author

Yes, I think that's the better approach. Then the test will be more realistic. Changes could still happen, but they would be at least relatively rare.

@dankohn
Copy link
Contributor

dankohn commented Aug 6, 2018 via email

@david-a-wheeler
Copy link
Collaborator Author

That test already uses some data from a ciitest project, but it didn't test the detected languages - because there weren't any!! I have added a Python file to the ciitest project, so we can have a more realistic test, and keeps the unit tests that try out other options. I'll push that version up soon.

So that resolves everything. Thanks!

This adds a direct test of the detected implementation languages
for the simple ciitest test project.

Signed-off-by: David A. Wheeler <dwheeler@dwheeler.com>
@david-a-wheeler david-a-wheeler merged commit 3204668 into master Aug 6, 2018
@david-a-wheeler david-a-wheeler deleted the detect_imp_lang branch August 6, 2018 03:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants