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

Fixups for golang-based entrypoint #136

Merged
merged 19 commits into from
Mar 16, 2022
Merged

Conversation

rohankh532
Copy link
Contributor

@rohankh532 rohankh532 commented Mar 9, 2022

  1. (Dockerfile) Fixed the dockerfile so that it uses the new golang entrypoint instead of entrypoint.sh update later
  2. (Makefile) Created a makefile for building scorecard-action
  3. (Dockerfile, options.go) Fixed the defaultScorecardPolicyFile not being copied update later
  4. (entrypoint.go) Changed resultsFilePath so that it is under the GithubWorkspace dir to fix file permission errors.
  5. (options.go) Properly pull & set EnvInputResultsFormat, EnvInputResultsFile, and EnvGithubAuthToken env vars
  6. (options_test.go) Set EnvInputResultsFormat and EnvInputResultsFile before calling options.New() to see if it properly picks up env vars.

@codecov
Copy link

codecov bot commented Mar 9, 2022

Codecov Report

Merging #136 (439b453) into main (e55a3ed) will increase coverage by 5.77%.
The diff coverage is 88.23%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #136      +/-   ##
==========================================
+ Coverage   69.02%   74.80%   +5.77%     
==========================================
  Files           2        2              
  Lines         113      127      +14     
==========================================
+ Hits           78       95      +17     
+ Misses         27       25       -2     
+ Partials        8        7       -1     
Impacted Files Coverage Δ
options/env.go 100.00% <ø> (ø)
options/options.go 74.40% <88.23%> (+5.93%) ⬆️

Copy link
Contributor

@azeemshaikh38 azeemshaikh38 left a comment

Choose a reason for hiding this comment

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

LGTM overall. Will let @justaugustus and @laurentsimon also take a look.

Dockerfile Outdated Show resolved Hide resolved
makefile Outdated Show resolved Hide resolved
makefile Outdated Show resolved Hide resolved
options/options_test.go Outdated Show resolved Hide resolved
Copy link
Member

@justaugustus justaugustus left a comment

Choose a reason for hiding this comment

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

Left a few initial pass comments!

makefile Outdated Show resolved Hide resolved
makefile Outdated Show resolved Hide resolved
options/options.go Outdated Show resolved Hide resolved
options/options.go Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
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 overall, added a few comments to clarify my understanding. Thanks!

entrypoint/entrypoint.go Show resolved Hide resolved
options/options.go Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
options/options_test.go Outdated Show resolved Hide resolved
Copy link
Member

@justaugustus justaugustus left a comment

Choose a reason for hiding this comment

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

@rohankh532 -- Left a few comments!

Makefile Outdated Show resolved Hide resolved
entrypoint/entrypoint.go Show resolved Hide resolved
options/env.go Outdated Show resolved Hide resolved
options/options.go Outdated Show resolved Hide resolved
options/options.go Outdated Show resolved Hide resolved
options/options.go Outdated Show resolved Hide resolved
@justaugustus
Copy link
Member

@rohankh532 -- I've got a few more review comments, but I'll issue them as pushes to your branch to speed things along!

@justaugustus justaugustus changed the title Scorecard build using entrypoint.go + other bugfixes Fixups for golang-based entrypoint Mar 16, 2022
@justaugustus justaugustus enabled auto-merge (squash) March 16, 2022 23:09
Copy link
Member

@justaugustus justaugustus left a comment

Choose a reason for hiding this comment

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

Thanks @rohankh532!!

@justaugustus justaugustus merged commit a46c05a into ossf:main Mar 16, 2022
justaugustus added a commit to justaugustus/scorecard that referenced this pull request May 26, 2022
- (Makefile) Created a makefile for building scorecard-action
- (entrypoint.go) Changed resultsFilePath so that it is under the GithubWorkspace dir to fix file permission errors
- (options.go) Properly pull & set EnvInputResultsFormat, EnvInputResultsFile, and EnvGithubAuthToken env vars
- (options_test.go) Set EnvInputResultsFormat and EnvInputResultsFile before calling options.New() to see if it properly picks up env vars.

Co-authored-by: Stephen Augustus <[email protected]>
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.

5 participants