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

✨ Feature: Dependency-diff API optimize: var re-naming, removing unused JSON tags #2090

Merged
merged 7 commits into from Jul 23, 2022

Conversation

aidenwang9867
Copy link
Contributor

What kind of change does this PR introduce?

  1. GetDependencydiffResults API (PR ✨ Feature DependencyDiff (Version 0 Part 2) #2046) input param optimization (renaming ambiguous ones).

  2. Remove unnecessary struct JSON tags for DependencyCheckResult: will define a JSONDependencyCheckResult struct with JSON tags in PR ✨ Feature [experimental]: The Scorecard Dependencydiff CLI (Version 0 Part 1) #2077.

What is the current behavior?

Var names such as baseSHA and headSHA might be confusing and ambiguous since users can use either SHAs or branch names as the inputs; changing them to base and head. There are some other minor changes on var names.

Struct JSON tags for DependencyCheckResult are unnecessary since we will define a JSONDependencyCheckResult struct with JSON tags in PR #2077 to output JSON. (removing these tags)

What is the new behavior (if this is a feature change)?**

  • Tests for the changes have been added (for bug fixes/features)

Which issue(s) this PR fixes

Special notes for your reviewer

Does this PR introduce a user-facing change?

For user-facing changes, please add a concise, human-readable release note to
the release-note

(In particular, describe what changes users might need to make in their
application as a result of this pull request.)

NONE

@codecov
Copy link

codecov bot commented Jul 22, 2022

Codecov Report

Merging #2090 (d6eda67) into main (0e4f5db) will increase coverage by 2.46%.
The diff coverage is 4.76%.

@@            Coverage Diff             @@
##             main    #2090      +/-   ##
==========================================
+ Coverage   42.16%   44.62%   +2.46%     
==========================================
  Files          92       92              
  Lines        7563     7569       +6     
==========================================
+ Hits         3189     3378     +189     
+ Misses       4126     3936     -190     
- Partials      248      255       +7     

Copy link
Contributor

@laurentsimon laurentsimon left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

@aidenwang9867 aidenwang9867 temporarily deployed to integration-test July 22, 2022 20:59 Inactive
@github-actions
Copy link

Integration tests success for
[9044f35]
(https://github.com/ossf/scorecard/actions/runs/2721149700)

@aidenwang9867 aidenwang9867 temporarily deployed to integration-test July 22, 2022 21:44 Inactive
@github-actions
Copy link

Integration tests success for
[d6eda67]
(https://github.com/ossf/scorecard/actions/runs/2721336439)

@laurentsimon laurentsimon merged commit 30e3f64 into ossf:main Jul 23, 2022
singhsaurabh pushed a commit to singhsaurabh/scorecard that referenced this pull request Jul 25, 2022
…ed JSON tags (ossf#2090)

* save

* save

* Update dependencydiff_result.go

* save

* save

* save
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

2 participants